fix(parser): track backtick identifiers; unify quote-aware SQL scanning#200
Open
alvarogar4 wants to merge 1 commit into
Open
fix(parser): track backtick identifiers; unify quote-aware SQL scanning#200alvarogar4 wants to merge 1 commit into
alvarogar4 wants to merge 1 commit into
Conversation
chkit pull dropped a table's projections and mangled its ORDER BY/PRIMARY KEY when a column had a backtick-quoted name containing a paren, e.g. `weird)name` (#196): the create-table body scanner counted that paren as structure and truncated the parse. Root cause was four separate copies of "scan SQL ignoring quoted regions" that had drifted apart — plugin-pull's tracked no quotes, the clickhouse body finder missed backticks (#197). They now share one quote-aware primitive, nextQuote, in @chkit/core, plus shared stripWrappingParens and findMatchingParen. splitTopLevelComma, the projection index normalizer, the pull key-clause parser, and the create-table body finder all handle '...', "...", and `...` identically, so the rule can't diverge again. Closes #196 Closes #197 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Biy2vG7iixRiBsBBsFg5Nu
6 tasks
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
1 participant
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.
Summary
Fixes #196 and #197 together, since #197 (the duplication) is the root cause of #196 (the bug).
#196 —
chkit pullsilently dropped projections on tables with a paren in a backtick column name.chkit's create-table body scanner tracked'and"but not backtick identifiers, so a column named`weird)name`had its)counted as structure, truncating the parsed column list. The table's projections (and a mangledORDER BY/PRIMARY KEY) followed — the same silent failure as #183. Verified live: ClickHouse accepts such a column, and the projection is now captured (was[]).#197 — four copies of "scan SQL, ignore quoted regions", already diverged.
splitTopLevelCommaand the projection stripper tracked all three quote kinds;plugin-pull'snormalizeWrappedTupletracked none; the clickhouse body finder missed backticks. They now share one primitive —nextQuotein@chkit/core— plus sharedstripWrappingParensandfindMatchingParen.plugin-pull's divergent copy is deleted. The rule lives in one place and can't drift again.Test plan
sql-scan.test.tslocks in quote/backtick behavior for all three shared helpersnextQuotefails 4 tests across the helpers and the parser (one fix point, enforced everywhere)typecheck/lint/buildclean`weird)name`column on ClickHouse 26.3 →listTableDetailsnow returns the projection (empty before)Notes
This consolidates the quote-state machine only; it does not change ClickHouse's expression formatting reconciliation, which is the separate, larger #195.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Biy2vG7iixRiBsBBsFg5Nu