Skip to content

Commit 50ee0a1

Browse files
committed
Fix parser issues: LIMIT/OFFSET order, hex literal conversion, update 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.)
1 parent 47bf571 commit 50ee0a1

5,846 files changed

Lines changed: 5884 additions & 5852 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

internal/explain/select.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,14 @@ func explainSelectQuery(sb *strings.Builder, n *ast.SelectQuery, indent string,
8787
Node(sb, o, depth+2)
8888
}
8989
}
90+
// OFFSET (ClickHouse outputs offset before limit in EXPLAIN AST)
91+
if n.Offset != nil {
92+
Node(sb, n.Offset, depth+1)
93+
}
9094
// LIMIT
9195
if n.Limit != nil {
9296
Node(sb, n.Limit, depth+1)
9397
}
94-
// OFFSET
95-
if n.Offset != nil {
96-
Node(sb, n.Offset, depth+1)
97-
}
9898
// SETTINGS - output here if there's no FORMAT, otherwise it's at SelectWithUnionQuery level
9999
if len(n.Settings) > 0 && n.Format == nil {
100100
fmt.Fprintf(sb, "%s Set\n", indent)

parser/expression.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -656,8 +656,19 @@ func (p *Parser) parseNumber() ast.Expression {
656656
value := p.current.Value
657657
p.nextToken()
658658

659-
// Check if it's a float
660-
if strings.Contains(value, ".") || strings.ContainsAny(value, "eE") {
659+
// Check if this is a hex, binary, or octal number
660+
isHex := strings.HasPrefix(value, "0x") || strings.HasPrefix(value, "0X")
661+
isBin := strings.HasPrefix(value, "0b") || strings.HasPrefix(value, "0B")
662+
isOctal := strings.HasPrefix(value, "0o") || strings.HasPrefix(value, "0O")
663+
664+
// Check for hex float (e.g., 0x1.2p3)
665+
isHexFloat := isHex && (strings.ContainsAny(value, "pP") || strings.Contains(value, "."))
666+
667+
// Check if it's a decimal float (but not a hex/binary/octal integer)
668+
// Note: hex numbers can contain 'e' as a hex digit, so we need to exclude them
669+
isDecimalFloat := !isHex && !isBin && !isOctal && (strings.Contains(value, ".") || strings.ContainsAny(value, "eE"))
670+
671+
if isDecimalFloat {
661672
f, err := strconv.ParseFloat(value, 64)
662673
if err != nil {
663674
lit.Type = ast.LiteralString
@@ -666,12 +677,33 @@ func (p *Parser) parseNumber() ast.Expression {
666677
lit.Type = ast.LiteralFloat
667678
lit.Value = f
668679
}
680+
} else if isHexFloat {
681+
// Parse hex float (Go doesn't support this directly, approximate)
682+
// For now, store as string - ClickHouse will interpret it
683+
lit.Type = ast.LiteralString
684+
lit.Value = value
669685
} else {
686+
// Determine the base for parsing
687+
// - 0x/0X: hex (base 16)
688+
// - 0b/0B: binary (base 2)
689+
// - 0o/0O: octal (base 8, explicit notation)
690+
// - Otherwise: decimal (base 10) - ClickHouse does NOT use leading zero for octal
691+
base := 10
692+
if isHex {
693+
base = 0 // Let strconv detect hex
694+
} else if isBin {
695+
base = 0 // Let strconv detect binary
696+
} else if isOctal {
697+
base = 0 // Let strconv detect octal with 0o prefix
698+
}
699+
// Note: We explicitly use base 10 for numbers like "077" because
700+
// ClickHouse does NOT interpret leading zeros as octal
701+
670702
// Try signed int64 first
671-
i, err := strconv.ParseInt(value, 10, 64)
703+
i, err := strconv.ParseInt(value, base, 64)
672704
if err != nil {
673705
// Try unsigned uint64 for large positive numbers
674-
u, uerr := strconv.ParseUint(value, 10, 64)
706+
u, uerr := strconv.ParseUint(value, base, 64)
675707
if uerr != nil {
676708
lit.Type = ast.LiteralString
677709
lit.Value = value
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

0 commit comments

Comments
 (0)