Fix parsing of a parenthesized left operand of a set operator - #312
Merged
git-hulk merged 4 commits intoAug 11, 2026
Merged
Conversation
parseSelectQuery bound UNION/EXCEPT/INTERSECT before consuming the
closing paren of a parenthesized select, so the operator never got a
chance to bind and ((SELECT 1) UNION ALL (SELECT 2)) failed with
"expected ')'". Parse a parenthesized operand by recursing into
parseSelectQuery and closing the paren first, then attach the operator's
right operand to the tail of the chain the left operand already carries.
Statements starting with '(' now dispatch to parseSelectQuery as well.
Fixes AfterShip#311
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Flattening a parenthesized operand into the existing chain changes semantics on the format round trip: ClickHouse gives INTERSECT higher precedence than UNION/EXCEPT, so (SELECT 1 UNION ALL SELECT 2) INTERSECT SELECT 2 returns 2 while its flattened form returns 1 and 2. A parenthesized operand is now kept as a SelectQuery with the new Paren field wrapping the inner query, so the operator after ')' binds to the whole group and FormatSQL reproduces the parentheses. parseCTEStmt consumes the parens around a CTE body itself so CTE ASTs are unchanged, and Paren marshals with omitempty, so no existing golden changes.
ClickHouse allows SETTINGS and FORMAT after the closing paren of a parenthesized select: (SELECT 1) SETTINGS max_threads=1 FORMAT JSONEachRow. The Paren group only bound set operators, so SETTINGS was rejected and a trailing FORMAT was consumed by parseStmt's discard call and silently dropped from the AST. Parse SETTINGS and FORMAT onto the group's own fields, mirroring the tail of parseSelectStmt, and have FormatSQL emit them. Once either clause is bound, no set operator may follow, matching ClickHouse, which rejects (SELECT 1) SETTINGS max_threads=1 UNION ALL SELECT 2.
Contributor
Author
|
Hey @git-hulk, |
Member
|
@therealpandey Thank you！Will have a look soon. |
Contributor
Author
|
Hey @git-hulk, |
git-hulk
reviewed
Aug 10, 2026
Member
|
@therealpandey Sorry for the late reply due to I'm on vacation those days. Overall is good to me, one comment inline. Thank you! |
Contributor
Author
|
Thanks @git-hulk. Addressed your comment. Appreciate the fast follow ups. |
git-hulk
approved these changes
Aug 11, 2026
Member
|
@therealpandey Thanks a lot for your contributions. |
This was referenced Aug 11, 2026
pull Bot
pushed a commit
to nagyist/signoz
that referenced
this pull request
Aug 12, 2026
### Description Bumps `github.com/AfterShip/clickhouse-sql-parser` from v0.5.5 to v0.5.6. - v0.5.6 parses a parenthesized left operand of a set operator (upstream AfterShip/clickhouse-sql-parser#312), e.g. `(SELECT 1) UNION ALL (SELECT 2)`. - Moves the three now-passing parenthesized set-operation cases into the pass table in `clickhouse_sql_test.go` as regression canaries. - Records the outstanding `NULLS FIRST|LAST` ORDER BY gap in the known-gap table — the parser still rejects it, so it stays tracked until fixed upstream.
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
2 participants
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.
Fixes #311
parseSelectQueryboundUNION/EXCEPT/INTERSECTbefore consuming the closing paren of a parenthesized select, so a set operator after)never got a chance to bind:All of these run on ClickHouse 26.8.1.337.
parseSelectQuerynow parses a parenthesized operand by recursing into itself and consuming)first, and keeps the group in the AST as aSelectQuerywith a newInnerQueryfield wrapping the inner query, so the operator after)binds to the whole group andFormatSQLreproduces the parentheses. The grouping has to survive into the AST: ClickHouse givesINTERSECThigher precedence thanUNION/EXCEPT, so flattening(SELECT 1 UNION ALL SELECT 2) INTERSECT SELECT 2into the paren-free chain would silently change the result set from2to1, 2on a parse→format round trip (see the discussion on #311).The group also binds
SETTINGSandFORMATafter the), which ClickHouse accepts —(SELECT 1) SETTINGS max_threads=1 FORMAT JSONEachRow— and once either is bound no set operator may follow, matching ClickHouse.InnerQuerymarshals withomitemptyandparseCTEStmtnow consumes the parens around a CTE body itself, so CTE ASTs and all existing goldens are unchanged.parseStmtadditionally dispatches statements starting with(toparseSelectQuery.Note for consumers: the few parenthesized-select inputs master already parsed by dropping the parens —
EXPLAIN SYNTAX (SELECT 1),ALTER TABLE ... MODIFY QUERY (SELECT 1),SELECT 1 UNION ALL (SELECT 2)— now produce theInnerQuerywrapper (with the inner query one level down andSelectItemsnil on the wrapper), andFormatpreserves their parentheses instead of normalizing them away.Covered by a fixture under
parser/testdata/query/including both INTERSECT precedence cases and the SETTINGS/FORMAT cases, an AST-shape test asserting the operator binds to the group, and invalid-syntax cases ((SELECT 1,(SELECT 1) UNION SELECT 2,(SELECT 1) SETTINGS max_threads=1 UNION ALL SELECT 2) that must keep failing. Every statement in the format golden round-trips to exactly its input text; regenerated goldens are new files only.