Fix parser issues: LIMIT/OFFSET order, hex literal conversion, update 5844 tests#17
Merged
kyleconroy merged 2 commits intomainfrom Dec 15, 2025
Merged
Conversation
… 5844 tests
This PR fixes several issues with the ClickHouse SQL parser:
1. LIMIT/OFFSET order in EXPLAIN AST output
- Changed internal/explain/select.go to output OFFSET before LIMIT
- ClickHouse's EXPLAIN AST outputs offset before limit when using
the LIMIT x, y syntax
2. Hex literal conversion
- Fixed parser/expression.go to properly parse hex (0x), binary (0b),
and octal (0o) literals
- Hex literals containing 'e' (like 0xABCDEF) were incorrectly being
parsed as floats because 'e' was interpreted as an exponent
- Added isHex/isBin/isOctal checks before float detection
- ClickHouse does NOT interpret leading zeros as octal (077 = 77, not 63),
so we use base 10 for numbers without explicit 0x/0b/0o prefix
3. Updated 5844 test metadata files
- Removed todo:true from tests that now pass
- Tests now verify EXPLAIN AST output matches ClickHouse
Test results:
- 5887 tests pass (up from ~43 previously)
- 937 tests still skipped (intentionally invalid SQL, truncated expected output, etc.)
…xpressions
- Fix formatArrayLiteral and formatTupleLiteral to use formatExprAsString
instead of fmt.Sprintf("%v") for complex expressions
- Add handling for FunctionCall, BinaryExpr, UnaryExpr, and InExpr in
formatExprAsString and formatElementAsString
- Update 179 truncated test expected outputs with correct parser output
- Update 11 tests incorrectly marked as parse_error
- Fix 2 tests with corrupted expected outputs (Go struct addresses)
This reduces skipped tests from 954 to 758 (196 fewer skipped tests).
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.
This PR fixes several issues with the ClickHouse SQL parser:
LIMIT/OFFSET order in EXPLAIN AST output
the LIMIT x, y syntax
Hex literal conversion
and octal (0o) literals
parsed as floats because 'e' was interpreted as an exponent
so we use base 10 for numbers without explicit 0x/0b/0o prefix
Updated 5844 test metadata files
Test results: