-
Notifications
You must be signed in to change notification settings - Fork 1
Fix INTERSECT/EXCEPT operator precedence in parser #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kyleconroy
wants to merge
24
commits into
main
Choose a base branch
from
claude/fix-tests-loop-IbPUK
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
INTERSECT has higher precedence than EXCEPT in ClickHouse, so: - "a EXCEPT b INTERSECT c" parses as "a EXCEPT (b INTERSECT c)" - "a INTERSECT b EXCEPT c" parses as "(a INTERSECT b) EXCEPT c" EXCEPT is also left-associative, creating binary trees: - "a EXCEPT b EXCEPT c" becomes "((a) EXCEPT b) EXCEPT c" This fixes 10 tests in 02004_intersect_except_operators and 10 tests in 02004_intersect_except_distinct_operators.
When a lambda is defined in a WITH clause like: WITH x -> x + 1 AS lambda SELECT lambda(1) The EXPLAIN AST output now correctly shows the alias: Function lambda (alias lambda) (children 1) This fixes 32 tests across multiple test files including: - 02343_analyzer_lambdas - 02344_analyzer_multiple_aliases_for_expression - 02366_explain_query_tree - 02378_analyzer_projection_names - 02388_analyzer_recursive_lambda - 02389_analyzer_nested_lambda - 03248_invalid_where - 03358_lambda_resolution_segfault_analyzer
When a lambda has an inline alias like: arrayMap((x -> toString(x)) as lambda, [1,2,3]) The EXPLAIN AST output now correctly shows the alias: Function lambda (alias lambda) (children 1) This fixes 9 more tests across: - 02343_analyzer_lambdas - 02378_analyzer_projection_names - 02389_analyzer_nested_lambda - 03101_analyzer_identifiers_2
Previously nested arrays with single-element arrays were rendered as Function array format, but they should be Literal format. Conversely, nested arrays containing tuples or empty arrays at any depth should use Function array format. Added recursive checks for empty arrays and tuples at any nesting depth to properly determine the correct EXPLAIN AST format. Fixes 33 test files with updated metadata.
The CREATE TABLE ... AS other_table ENGINE=... syntax wasn't parsing PARTITION BY, ORDER BY, SAMPLE BY, or SETTINGS clauses after the ENGINE specification. Refactored table options parsing into parseTableOptions function and call it after AS clause ENGINE parsing. Fixes 14 test files with CREATE TABLE AS syntax.
- Added YYYY as an alias for YEAR in EXTRACT field parsing - ExtractExpr now properly uses its Alias field for EXPLAIN output - Added ExtractExpr case in explainAliasedExpr for alias wrapping Fixes 00619_extract test.
CASE expressions without an explicit ELSE clause implicitly return NULL. Update the explain output to always include this implicit NULL value, matching ClickHouse's EXPLAIN AST format. Fixes 4 statements in 4 tests.
When a tuple contains only primitive literals (including nested tuples with primitive literals), it should be rendered as a single Literal Tuple_(...) rather than Function tuple with children. Add containsOnlyPrimitiveLiteralsWithUnary helper that checks if a tuple contains only primitive literals, including handling unary negation of numeric literals (e.g., -0., -123). Fixes 17 statements across 11 tests.
When ALTER TABLE MODIFY COLUMN specifies only a CODEC without a data type (e.g., `MODIFY COLUMN col CODEC(LZ4)`), the parser was incorrectly treating CODEC as the column type. Now correctly recognizes CODEC as a codec specification when no type is present. Fixes 10 statements across 5 tests.
The Column function was missing handling for column-level TTL expressions. Now properly includes the TTL expression in the ColumnDeclaration's children. Fixes 75 statements across 26 tests.
The MODIFY_TTL alter command's TTL expression should be wrapped in ExpressionList and TTLElement nodes to match ClickHouse's EXPLAIN AST format. Fixes 32 statements across 14 tests.
Each grouping set element in GROUPING SETS clause needs to be wrapped in an ExpressionList in the EXPLAIN output. Added GroupingSets flag to SelectQuery AST to track when GROUPING SETS is used, and updated the explain code to output the wrapper when needed. Fixes 66 statements across 25 tests.
When EXPLAIN wraps a SELECT query with FORMAT clause, the FORMAT identifier should be a child of the Explain node, not the inner SelectWithUnionQuery. Updated explainExplainQuery to extract the format from the inner SelectQuery and output it as a sibling. Fixes 17 statements across 13 tests.
Remove incorrect maxStringTupleSize limit that prevented large string lists in IN expressions from being combined into a single Tuple literal. ClickHouse EXPLAIN AST always outputs IN lists of strings as a single Literal Tuple_(...) regardless of size. Fixes 5 tests.
When outputting GROUPING SETS elements, unwrap tuple literals to output their elements directly with correct child counts instead of wrapping in ExpressionList(children 1) and outputting as Function tuple. Fixes tests for GROUPING SETS queries across 12 test files.
- Fixed GROUPING SETS handling to unwrap tuple literals and output elements directly with correct child counts - Added TOP clause support to SELECT parser (use MUL_PREC to stop at *) - Added TOP clause output to EXPLAIN AST at end of SelectQuery - Updated 3 GROUPING SETS tests that now pass The TOP fix prevents `SELECT TOP 5 * FROM t` from being misparsed as `TOP (5 * FROM t)` where * was treated as multiplication.
00834_limit_with_constant_expressions stmt16 (SELECT TOP) now passes after the TOP clause parsing and EXPLAIN support added in previous commit.
ClickHouse allows join strictness modifiers (ANTI, SEMI, ANY, ALL, ASOF) either before or after the join type. This fix adds support for parsing strictness after the type, enabling queries like: - RIGHT ANTI JOIN - LEFT SEMI JOIN etc. 10 tests now pass with this fix.
Add parsing of column list after view name in CREATE VIEW, e.g.: CREATE VIEW v (x UInt64) AS SELECT * FROM table This enables views to specify column types for the view columns. 14 tests now pass with this fix.
Parse table options (ORDER BY, PRIMARY KEY, etc.) after ENGINE for materialized views. Fixes 26 tests including stmt2 in test 03667.
1. Fix WITH clause to only treat ( as subquery when followed by SELECT/WITH Previously (SELECT...) AS name was parsed even for empty () or expressions 2. Fix empty tuple EXPLAIN output to use Function tuple format Empty tuples should output "Function tuple (children 1)" with ExpressionList rather than "Literal Tuple_()" Fixes 41 tests including materialized views and WITH clause tests.
- Added Settings field to AlterQuery struct in ast/ast.go - Parse SETTINGS clause after ALTER commands in parser/parser.go - Output Set child in explainAlterQuery when Settings are present This fixes ~90 tests that use ALTER TABLE with SETTINGS clause like: ALTER TABLE t ADD COLUMN c String SETTINGS alter_sync = 2
When a lambda has no parameters (e.g., `() -> expr`), ClickHouse outputs just `ExpressionList` without the `(children N)` suffix. Previously we were outputting `ExpressionList (children 0)` which didn't match the expected format. Fixes 5 tests related to user-defined functions with empty lambda args.
When ORDER BY has WITH fill step N (without FROM/TO), ClickHouse outputs the step value directly as a Literal child of OrderByElement, not wrapped in a FillModifier node. Previously we used FillModifier for step-only cases, but ClickHouse only uses FillModifier when FROM/TO contain complex expressions. Fixes 19+ statements across 6 tests related to WITH FILL.
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.
INTERSECT has higher precedence than EXCEPT in ClickHouse, so:
EXCEPT is also left-associative, creating binary trees:
This fixes 10 tests in 02004_intersect_except_operators and 10 tests
in 02004_intersect_except_distinct_operators.