Repository navigation
fix: canonicalize SQL values while preserving source spans and quoted names - #66
Merged
Merged
Conversation
Signed-off-by: Snehil Shah <snehilshah.989@gmail.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Comment |
Snehil-Shah
commented
Aug 29, 2026
Contributor
Author
There was a problem hiding this comment.
@renecannao I added a dedicated file as an AST-level regression suite for this. Let me know if you want it spread across existing files instead.
Signed-off-by: Snehil Shah <snehilshah.989@gmail.com>
Merge current main into the canonicalization branch without rewriting the original contribution's history. Port its keyword, function-name, join, ordering, and system-variable normalization to the grammar added in ProxySQL#67. Keep existing PostgreSQL productions, quoted identifiers, aggregate modifiers, and MySQL function-spacing flags intact. Canonical function and variable values may live in arena storage, so they must not be used as input locations. Record original source spans for function calls and system-variable references, and restrict unary span construction to addresses within the tokenizer input. This fixes cases such as SELECT -abs(1), which previously reported a source length far beyond the SQL buffer after the name was copied into the arena. Preserve PostgreSQL quoted function components byte-for-byte while folding unquoted names to lowercase, including qualified names separated by comments or whitespace. Fold MySQL function names to uppercase while retaining backticks. Normalize system-variable separators without losing identifier quotes. Keep JOIN reconstruction bounded and report allocation failure instead of returning incomplete canonical text. Avoid allocating another string for names already in canonical case, and report failed case-fold allocations. Reject missing MySQL function arguments when the arena is exhausted rather than returning a successful AST with the argument silently removed. Add regressions for source bounds and exact source text, qualified and quoted function names, system-variable quoting, arena exhaustion, and allocation-free canonical names. Update two existing emission expectations for the intentional dialect-specific function casing without changing their argument/placeholder coverage. Validation: full test suite passes 1,732 tests with 37 backend-dependent integration tests skipped; corpus harness builds; all 25 canonicalization tests also pass with AddressSanitizer and UndefinedBehaviorSanitizer. The new source-span and allocation tests were first run failing against the corresponding unfixed implementations. No benchmark artifacts added. Refs: ProxySQL#66
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.
Canonicalizes expression keyword operators, SELECT/order/locking/set-operation keywords, JOIN separators, ordinary function names, and system-variable separators so equivalent spelling and whitespace produce stable AST values and digests.
MySQL function names fold to uppercase while retaining backtick delimiters. PostgreSQL unquoted function-name components fold to lowercase; quoted components retain their exact spelling, including in qualified names. The branch incorporates current main and preserves its grammar, aggregate modifiers, and MySQL function-spacing distinctions.
Canonical values are kept separate from original source locations. Function calls and system-variable references retain input spans, and unary span construction checks input bounds. This prevents arena-owned normalized strings from producing out-of-bounds source spans. Allocation failures report errors, including failures while parsing MySQL function arguments; already canonical names reuse their existing storage.
Validation:
make test: 1,732 passed, 37 backend-dependent integration tests skipped, zero failures.make build-corpus-test: passed.make test-mysql-compat: 545 curated witnesses matched expectations; no live MySQL oracle was used.make test-pg-compat: 87 harness tests and 5,122 committed compatibility cases passed.The original PR history is preserved through a merge of current main and a detailed repair commit. No benchmark artifacts are included.