Repository navigation
perf: evaluate built-in rules over Polyglot query facts and drop sqlparser - #1143
Merged
Merged
Conversation
Rules need analysis's Snowflake variant normalization together with the character map back to the authored SQL, so positions survive the rewrite.
Built-in model and SQL-test rules re-parsed every model with sqlparser. They now read compact facts from one Polyglot parse per stubbed query shape: dependency calls are stubbed by occurrence, shapes are shared by the rules workers when their stubbed SQL is byte-identical, and positions map back to each model's own SQL. Models sqlparser rejected but Polyglot parses are now evaluated; SQBRDECLARATION101/102 quote the comparison's source text and follow its source position. Unparseable SQL keeps the unevaluated result with Polyglot's error.
The rules quote policy is a static dialect table, so no crate needs sqlparser any more.
The stub scanner only knew -- and /* */ comments, so a Snowflake // or a Snowflake/BigQuery # comment could hold a phantom __ref call, or an apostrophe in it could open a fake string that swallowed real calls. Scan with the comment and quoting syntax the rules parser used per dialect, and blank comments before normalization and parsing.
…ar facts - SQBRDECLARATION101 compares each string literal's own source text with the authored SQL and covers double-quoted, triple-quoted, escape and Unicode strings again. - SQBRTEST203 no longer treats a parameterized cast type such as DECIMAL(18, 2) as a function call. - A T-SQL TOP (n), TOP n PERCENT or WITH TIES clause is not projection. - Stars in dependency import CTEs are exempt from SQBRMODEL102 by CTE classification, not by having a recovered source position.
A DuckDB FROM-first query has an implicit star but no SELECT token, so its star finding fell back to 1:1. Anchor that star on the FROM keyword. A SELECT whose only located part is an unaliased subquery is now matched to its own SELECT keyword through the subquery's opening parenthesis, so its star keeps its source position too.
SQBRTEST203 counted the leading parameterized type of a T-SQL CONVERT(VARCHAR(10), x) or TRY_CONVERT(DECIMAL(18, 2), x) as a function call, so an empty SQL Server mock or bare existence assertion was not flagged. Skip that type argument, as sqlparser's conversion parse did; function calls in the value argument still count.
kvlonge
enabled auto-merge (squash)
October 18, 2026 11:34
…okens
A set-operation branch whose items carry no Polyglot span, such as
SELECT CAST(1 AS INTEGER), CAST('games' AS VARCHAR) after an aliased
first branch, found no SELECT token region. Its SQBRCONTRACT105 cast
types then fell back to Polyglot's spelling and its findings to 1:1,
producing false contract findings. Such a select now takes the next
unclaimed SELECT region after the previous branch, since branches and
CTEs are read in source order.
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.
Why
Built-in model rules re-parsed every model with a second SQL parser (sqlparser), about 80% of built-in rules CPU on the dense 10k project, while the compiler already uses Polyglot. Keeping two parsers also meant rules and the compiler could disagree about what parses.
Changes
SELECT * FROM (subquery) UNION ALL ...follows a Polyglot parse bug (CHI-627).Verification
rules run SQBRfindings identical between base and candidate on every corpus compared (600-model project: 7,794; examples, fixtures and synthetic DuckDB/Snowflake edge-case projects).