Repository navigation
perf(sql): read the confirmation gate's text in linear time - #2249
Merged
Merged
Conversation
…EXPLAIN The confirmation gate read the statement an EXPLAIN executes by calling itself on it, so a PostgreSQL text of EXPLAIN ANALYZE repeated ran V8 out of stack: the web editor, the VS Code extension's child and a Node server threw a RangeError from a 250,000-unit chain, short of the 1,000,000 units the statement analysis route accepts. The chain is now followed in a loop, which is the same tail call written out, so every verdict stays as it was. Bun's engine eliminates the tail call and never showed it, so the extension's query/target suite pins the depth under Node.
…e units Every reader asks whether an `r` on Spark continues a word, and the answer split the two code units before it into an array of code points to take the last one. On 1,000,000 units of `r` words that was most of the confirmation gate's 1.5 s. The code point is now read off the two code units: a surrogate pair whole, any other unit alone, as the array gave it.
…record `readLeadingKeyword` reads a leading `#` as a comment on every dialect, and it copied the caller's grammar record to say so on every call. The confirmation gate reads the leading keyword of every statement of a script, so a script of 100,000 short statements made 100,000 copies. The copy is now kept per record, which is a constant, so each dialect is copied once.
…n per nested or repeated form Four checks read the rest of the text again for every copy of what they looked for, so their cost grew with the square of the text, minutes at the 1,000,000 units the statement analysis route accepts: - a CTE body that opens with WITH had its list read again from that WITH, crossing every body nested inside it once per level (65 s for 250,000 units of nested bodies on PostgreSQL); - a Spark computed call looked for the end of its run, crossing every call nested inside it again (68 s for 250,000 units); - a Snowflake CREATE STAGE looked for URL to the end of the text after every occurrence (52 s for 250,000 units); - a FOR UPDATE lock clause in a block matched its pattern against the whole text before every UPDATE (40 s for 1,000,000 units). Where a parenthesised run closes is now remembered for every index a walk stood on (ParenthesisedRuns), which is exact because a walk's next step depends only on its index, and the nested CTE lists are read in place to share it. A conditional form is decided at the first occurrence its words follow, and the lock clause pattern reads the white space before the UPDATE and the four characters before that. Every verdict stays as it was: the existing gate tests, the analysis parity tests, and a probe comparing the old gate with this one over 400,000 generated texts and every connection type agree.
…ion gate asks about The gate walked the text once for every confirmation form and shared word it looked for, and read the words after every occurrence once per form that starts with that word: about 100 walks of a Snowflake block and more than 200 of a block of CREATE words. One walk now indexes the code words, prefixes and quoted names those forms are found by (indexCodeWords), word sequences that share their first word are compared as one tree, a quoted call's names as one set, and a grammar's forms are compiled once per record. A statement that is the whole text, as one under the caret is, is not read a second time. Measured on 1,000,000 units under Bun 1.4.4: 166,666 Snowflake BEGIN words 1065 ms to 40 ms, Snowflake CREATE words 2.8 s to 94 ms, 500,000 Spark words 649 ms to 69 ms, 100,000 Spark statements 645 ms to 114 ms. Every verdict stays as it was: the gate tests, the analysis parity tests and the old-against-new probe over 400,000 generated texts and every connection type agree.
…n gate is linear The route's bound comment, the known limit in docs/SECURITY.md and backlog U202 named 1.2 s for 1,000,000 units on Snowflake, which the gate no longer costs. They now state the 2026-11-05 measurements: about 45 ms for a PostgreSQL script and up to about 0.22 s for the slowest texts found, of which the gate is at most about 0.2 s, so one user can still ask for about 26 s of CPU in every minute. U202 stays open, narrowed to that residual: its done-when asks for a charge that grows with the text or a gate near the splitter's 14 ms, and neither is met. S12 records a sibling found while measuring: the row limiter's CTE body reader is quadratic in nested bodies and recursive per level.
…er closure The CTE list reader the splitter reaches through operative-keyword.ts now imports parenthesised.ts, which reaches only spans.ts and words.ts, so the closure the browser pays for gains one pure reader.
…sh on a slow runner Two million-unit texts under c8 take about 1.2 s locally and went past mocha's 2 s default on a CI runner. The gate's growth is pinned by tests/unit/sql/dangerous-query.test.ts, so this test only needs time to finish, not a tighter bound.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
cevheri
added a commit
that referenced
this pull request
Nov 5, 2026
…unds sized to their bound (#2256) * test(sql): time the confirmation gate's cost guards in alternating rounds sized to their bound The cost guards divided the cheapest of five runs of one side by the cheapest of five of the other, timed one after the other. A 1 ms walk often escapes preemption whole while a 12 ms gate run cannot, so a busy macOS runner read a script of short statements at 34.8 walks against a bound of 24, for a gate that costs about 12; the growth guard shares the flaw. Each guard now times one gate run, or two of the long text, against 24 times as many yardstick runs, in nine rounds that alternate which side goes first, and reads the median round, so both sides of a round take the same load where the cost meets the bound, which stays 24. On one CPU shared with two other test processes the old guards failed every run of the file and these none, while the gate before #2249 still reads above 30 on every shape. * test(sql): state what the cost guards' timeout and alternating rounds were measured to do
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SQL confirmation gate (
isDangerousQuery) cost the square of the text on several shapes, soPOST /api/db/statement/analyzeat its 1,000,000-unit bound could take seconds (nested CTE bodies on PostgreSQL: 65 s at 250,000 units). It is linear now on every shape measured, with no decision changed. The same gate runs in the web editor and the VS Code extension, so they gain too.indexCodeWords), each dialect's forms compile once per grammar, nested parenthesised runs and CTE lists are read in place with remembered closes (src/lib/sql/parenthesised.ts), and the EXPLAIN chain is followed in a loop.BEGINwords 1039 ms to 38 ms, nested closed CTEs on SQL Server 71 s (at 500k) to 107 ms. The slowest shapes stay at 100 to 200 ms, so U202 stays open, narrowed to that residual (a size-based charge is a contract decision of S-06: POST /api/db/statement/analyze, the web editor's run decision for clients outside the browser #2204). S12 records the row limiter's own quadratic CTE reader, which this does not touch.isDangerousQueryandhasUnreadableRun, every engine type) over about 180 million checks of random, adversarial, crafted and mutated texts found 0 mismatches. Fault injection showed the probes catch the regressions they target.Gate set,
build:libandattw, coverage 100%, and the extension's build, unit and coverage legs green locally.