Skip to content

Commit d82d2e1

Browse files
kyleconroykyleconroy
authored
Fix 10 parser tests with various EXPLAIN AST improvements (#43)
1 parent 34c8a4c commit d82d2e1

30 files changed

Lines changed: 156 additions & 56 deletions

File tree

‎internal/explain/expressions.go‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,27 @@ func explainLiteral(sb *strings.Builder, n *ast.Literal, indent string, depth in
4949
}
5050
hasComplexExpr := false
5151
for _, e := range exprs {
52-
lit, isLit := e.(*ast.Literal)
53-
// Non-literals or tuple/array literals count as complex
54-
if !isLit || (isLit && (lit.Type == ast.LiteralTuple || lit.Type == ast.LiteralArray)) {
55-
hasComplexExpr = true
56-
break
52+
// Simple literals (numbers, strings, etc.) are OK
53+
if lit, isLit := e.(*ast.Literal); isLit {
54+
// Nested tuples/arrays are complex
55+
if lit.Type == ast.LiteralTuple || lit.Type == ast.LiteralArray {
56+
hasComplexExpr = true
57+
break
58+
}
59+
// Other literals are simple
60+
continue
61+
}
62+
// Unary negation of numeric literals is also simple
63+
if unary, isUnary := e.(*ast.UnaryExpr); isUnary && unary.Op == "-" {
64+
if lit, isLit := unary.Operand.(*ast.Literal); isLit {
65+
if lit.Type == ast.LiteralInteger || lit.Type == ast.LiteralFloat {
66+
continue
67+
}
68+
}
5769
}
70+
// Everything else is complex
71+
hasComplexExpr = true
72+
break
5873
}
5974
if hasComplexExpr {
6075
// Render as Function tuple instead of Literal
@@ -329,6 +344,33 @@ func explainAliasedExpr(sb *strings.Builder, n *ast.AliasedExpr, depth int) {
329344
}
330345
}
331346
}
347+
// Check if this is an array containing specific expressions that need Function array format
348+
if e.Type == ast.LiteralArray {
349+
if exprs, ok := e.Value.([]ast.Expression); ok {
350+
needsFunctionFormat := false
351+
for _, expr := range exprs {
352+
// Check for tuples - use Function array
353+
if lit, ok := expr.(*ast.Literal); ok && lit.Type == ast.LiteralTuple {
354+
needsFunctionFormat = true
355+
break
356+
}
357+
// Check for identifiers - use Function array
358+
if _, ok := expr.(*ast.Identifier); ok {
359+
needsFunctionFormat = true
360+
break
361+
}
362+
}
363+
if needsFunctionFormat {
364+
// Render as Function array with alias
365+
fmt.Fprintf(sb, "%sFunction array (alias %s) (children %d)\n", indent, n.Alias, 1)
366+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(exprs))
367+
for _, expr := range exprs {
368+
Node(sb, expr, depth+2)
369+
}
370+
return
371+
}
372+
}
373+
}
332374
fmt.Fprintf(sb, "%sLiteral %s (alias %s)\n", indent, FormatLiteral(e), n.Alias)
333375
case *ast.BinaryExpr:
334376
// Binary expressions become functions with alias
@@ -450,6 +492,8 @@ func explainWithElement(sb *strings.Builder, n *ast.WithElement, indent string,
450492
fmt.Fprintf(sb, "%sSubquery (children %d)\n", indent, 1)
451493
}
452494
Node(sb, e.Query, depth+1)
495+
case *ast.CastExpr:
496+
explainCastExprWithAlias(sb, e, n.Name, indent, depth)
453497
default:
454498
// For other types, just output the expression (alias may be lost)
455499
Node(sb, n.Query, depth)

‎internal/explain/format.go‎

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,14 @@ func FormatFloat(val float64) string {
2121
if math.IsNaN(val) {
2222
return "nan"
2323
}
24-
// Use 'f' format to avoid scientific notation, -1 precision for smallest representation
24+
// Use scientific notation for extremely small numbers (< 1e-10)
25+
// This matches ClickHouse's behavior where numbers like 0.000001 stay decimal
26+
// but extremely small numbers like 1e-38 use scientific notation
27+
absVal := math.Abs(val)
28+
if absVal > 0 && absVal < 1e-10 {
29+
return strconv.FormatFloat(val, 'e', -1, 64)
30+
}
31+
// Use decimal notation for normal-sized numbers
2532
return strconv.FormatFloat(val, 'f', -1, 64)
2633
}
2734

@@ -218,35 +225,65 @@ func FormatDataType(dt *ast.DataType) string {
218225
} else if ntp, ok := p.(*ast.NameTypePair); ok {
219226
// Named tuple field: "name Type"
220227
params = append(params, ntp.Name+" "+FormatDataType(ntp.Type))
228+
} else if binExpr, ok := p.(*ast.BinaryExpr); ok {
229+
// Binary expression (e.g., 'hello' = 1 for Enum types)
230+
params = append(params, formatBinaryExprForType(binExpr))
221231
} else {
222232
params = append(params, fmt.Sprintf("%v", p))
223233
}
224234
}
225235
return fmt.Sprintf("%s(%s)", dt.Name, strings.Join(params, ", "))
226236
}
227237

238+
// formatBinaryExprForType formats a binary expression for use in type parameters
239+
func formatBinaryExprForType(expr *ast.BinaryExpr) string {
240+
var left, right string
241+
242+
// Format left side
243+
if lit, ok := expr.Left.(*ast.Literal); ok {
244+
if lit.Type == ast.LiteralString {
245+
left = fmt.Sprintf("\\\\\\'%s\\\\\\'", lit.Value)
246+
} else {
247+
left = fmt.Sprintf("%v", lit.Value)
248+
}
249+
} else if ident, ok := expr.Left.(*ast.Identifier); ok {
250+
left = ident.Name()
251+
} else {
252+
left = fmt.Sprintf("%v", expr.Left)
253+
}
254+
255+
// Format right side
256+
if lit, ok := expr.Right.(*ast.Literal); ok {
257+
right = fmt.Sprintf("%v", lit.Value)
258+
} else if ident, ok := expr.Right.(*ast.Identifier); ok {
259+
right = ident.Name()
260+
} else {
261+
right = fmt.Sprintf("%v", expr.Right)
262+
}
263+
264+
return left + " " + expr.Op + " " + right
265+
}
266+
228267
// NormalizeFunctionName normalizes function names to match ClickHouse's EXPLAIN AST output
229268
func NormalizeFunctionName(name string) string {
230269
// ClickHouse normalizes certain function names in EXPLAIN AST
231270
normalized := map[string]string{
232-
"ltrim": "trimLeft",
233-
"rtrim": "trimRight",
234-
"lcase": "lower",
235-
"ucase": "upper",
236-
"mid": "substring",
237-
"ceiling": "ceil",
238-
"ln": "log",
239-
"log10": "log10",
240-
"log2": "log2",
241-
"rand": "rand",
242-
"ifnull": "ifNull",
243-
"nullif": "nullIf",
244-
"coalesce": "coalesce",
245-
"greatest": "greatest",
246-
"least": "least",
247-
"concat_ws": "concat",
248-
"length": "length",
249-
"char_length": "length",
271+
"ltrim": "trimLeft",
272+
"rtrim": "trimRight",
273+
"lcase": "lower",
274+
"ucase": "upper",
275+
"mid": "substring",
276+
"ceiling": "ceil",
277+
"ln": "log",
278+
"log10": "log10",
279+
"log2": "log2",
280+
"rand": "rand",
281+
"ifnull": "ifNull",
282+
"nullif": "nullIf",
283+
"coalesce": "coalesce",
284+
"greatest": "greatest",
285+
"least": "least",
286+
"concat_ws": "concat",
250287
}
251288
if n, ok := normalized[strings.ToLower(name)]; ok {
252289
return n

‎internal/explain/functions.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ func explainFunctionCallWithAlias(sb *strings.Builder, n *ast.FunctionCall, alia
2121
}
2222
// Normalize function name
2323
fnName := NormalizeFunctionName(n.Name)
24+
// Append "Distinct" if the function has DISTINCT modifier
25+
if n.Distinct {
26+
fnName = fnName + "Distinct"
27+
}
2428
if alias != "" {
2529
fmt.Fprintf(sb, "%sFunction %s (alias %s) (children %d)\n", indent, fnName, alias, children)
2630
} else {
@@ -774,7 +778,6 @@ func explainExtractExpr(sb *strings.Builder, n *ast.ExtractExpr, indent string,
774778
func explainWindowSpec(sb *strings.Builder, n *ast.WindowSpec, indent string, depth int) {
775779
// Window spec is represented as WindowDefinition
776780
// For simple cases like OVER (), just output WindowDefinition without children
777-
// Note: ClickHouse's EXPLAIN AST does not output frame info (ROWS BETWEEN etc)
778781
children := 0
779782
if n.Name != "" {
780783
children++
@@ -785,6 +788,10 @@ func explainWindowSpec(sb *strings.Builder, n *ast.WindowSpec, indent string, de
785788
if len(n.OrderBy) > 0 {
786789
children++
787790
}
791+
// Count frame offset as child if present
792+
if n.Frame != nil && n.Frame.StartBound != nil && n.Frame.StartBound.Offset != nil {
793+
children++
794+
}
788795
if children > 0 {
789796
fmt.Fprintf(sb, "%sWindowDefinition (children %d)\n", indent, children)
790797
if n.Name != "" {
@@ -802,7 +809,10 @@ func explainWindowSpec(sb *strings.Builder, n *ast.WindowSpec, indent string, de
802809
explainOrderByElement(sb, o, strings.Repeat(" ", depth+2), depth+2)
803810
}
804811
}
805-
// Frame handling would go here if needed
812+
// Frame start offset
813+
if n.Frame != nil && n.Frame.StartBound != nil && n.Frame.StartBound.Offset != nil {
814+
Node(sb, n.Frame.StartBound.Offset, depth+1)
815+
}
806816
} else {
807817
fmt.Fprintf(sb, "%sWindowDefinition\n", indent)
808818
}

‎lexer/lexer.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,19 @@ func (l *Lexer) readQuotedIdentifier() Item {
429429
var sb strings.Builder
430430
l.readChar() // skip opening quote
431431

432-
for !l.eof && l.ch != '"' {
432+
for !l.eof {
433+
if l.ch == '"' {
434+
// Check for SQL-style doubled quote escape ""
435+
l.readChar()
436+
if l.ch == '"' {
437+
// Doubled quote - add single quote and continue
438+
sb.WriteRune('"')
439+
l.readChar()
440+
continue
441+
}
442+
// Single quote - end of identifier
443+
break
444+
}
433445
if l.ch == '\\' {
434446
l.readChar()
435447
if !l.eof {
@@ -441,9 +453,6 @@ func (l *Lexer) readQuotedIdentifier() Item {
441453
sb.WriteRune(l.ch)
442454
l.readChar()
443455
}
444-
if l.ch == '"' {
445-
l.readChar() // skip closing quote
446-
}
447456
return Item{Token: token.IDENT, Value: sb.String(), Pos: pos}
448457
}
449458

‎parser/testdata/00160_decode_xml_component/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

‎parser/testdata/00267_tuple_array_access_operators_priority/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

‎parser/testdata/00317_in_tuples_and_out_of_range_values/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

‎parser/testdata/00324_hashing_enums/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

‎parser/testdata/00471_sql_style_quoting/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

‎parser/testdata/00502_string_concat_with_array/metadata.json‎

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)