Skip to content

Commit 0ab2992

Browse files
kyleconroykyleconroy
authored
Fix 7 parser tests by improving explain output (#57)
1 parent 00c905a commit 0ab2992

File tree

23 files changed

+98
-41
lines changed

23 files changed

+98
-41
lines changed

‎internal/explain/expressions.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ func explainLiteral(sb *strings.Builder, n *ast.Literal, indent string, depth in
4747
fmt.Fprintf(sb, "%s ExpressionList\n", indent)
4848
return
4949
}
50+
// Single-element tuples (from trailing comma syntax like (1,)) always render as Function tuple
51+
if len(exprs) == 1 {
52+
fmt.Fprintf(sb, "%sFunction tuple (children %d)\n", indent, 1)
53+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(exprs))
54+
for _, e := range exprs {
55+
Node(sb, e, depth+2)
56+
}
57+
return
58+
}
5059
hasComplexExpr := false
5160
for _, e := range exprs {
5261
// Simple literals (numbers, strings, etc.) are OK
@@ -454,11 +463,16 @@ func explainAliasedExpr(sb *strings.Builder, n *ast.AliasedExpr, depth int) {
454463
explainCastExpr(sb, e, indent, depth)
455464
}
456465
case *ast.ArrayAccess:
457-
// Array access - ClickHouse doesn't show aliases on arrayElement in EXPLAIN AST
458-
explainArrayAccess(sb, e, indent, depth)
466+
// Array access - show alias only when array is not a literal
467+
// ClickHouse hides alias when array access is on a literal
468+
if _, isLit := e.Array.(*ast.Literal); isLit {
469+
explainArrayAccess(sb, e, indent, depth)
470+
} else {
471+
explainArrayAccessWithAlias(sb, e, n.Alias, indent, depth)
472+
}
459473
case *ast.TupleAccess:
460-
// Tuple access - ClickHouse doesn't show aliases on tupleElement in EXPLAIN AST
461-
explainTupleAccess(sb, e, indent, depth)
474+
// Tuple access with alias
475+
explainTupleAccessWithAlias(sb, e, n.Alias, indent, depth)
462476
case *ast.InExpr:
463477
// IN expressions with alias
464478
explainInExprWithAlias(sb, e, n.Alias, indent, depth)

‎internal/explain/functions.go‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -395,19 +395,20 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
395395
argCount += len(n.List)
396396
}
397397
} else {
398-
// Check if all items are tuples
399-
allTuples := true
398+
// Check if all items are string literals (large list case - no wrapper)
399+
allStringLiterals := true
400400
for _, item := range n.List {
401-
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralTuple {
402-
allTuples = false
401+
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
402+
allStringLiterals = false
403403
break
404404
}
405405
}
406-
if allTuples {
407-
// All tuples get wrapped in a single Function tuple
408-
argCount++
409-
} else {
406+
if allStringLiterals {
407+
// Large string list - separate children
410408
argCount += len(n.List)
409+
} else {
410+
// Non-string items get wrapped in a single Function tuple
411+
argCount++
411412
}
412413
}
413414
}
@@ -455,8 +456,26 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
455456
explainTupleInInList(sb, item.(*ast.Literal), indent+" ", depth+4)
456457
}
457458
} else {
459+
// Check if all items are string literals (large list case)
460+
allStringLiterals := true
458461
for _, item := range n.List {
459-
Node(sb, item, depth+2)
462+
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
463+
allStringLiterals = false
464+
break
465+
}
466+
}
467+
if allStringLiterals {
468+
// Large string list - output as separate children (no tuple wrapper)
469+
for _, item := range n.List {
470+
Node(sb, item, depth+2)
471+
}
472+
} else {
473+
// Wrap non-literal/non-tuple list items in Function tuple
474+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
475+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.List))
476+
for _, item := range n.List {
477+
Node(sb, item, depth+4)
478+
}
460479
}
461480
}
462481
}
@@ -548,18 +567,20 @@ func explainInExprWithAlias(sb *strings.Builder, n *ast.InExpr, alias string, in
548567
argCount += len(n.List)
549568
}
550569
} else {
551-
// Check if all items are tuples
552-
allTuples := true
570+
// Check if all items are string literals (large list case - no wrapper)
571+
allStringLiterals := true
553572
for _, item := range n.List {
554-
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralTuple {
555-
allTuples = false
573+
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
574+
allStringLiterals = false
556575
break
557576
}
558577
}
559-
if allTuples {
560-
argCount++
561-
} else {
578+
if allStringLiterals {
579+
// Large string list - separate children
562580
argCount += len(n.List)
581+
} else {
582+
// Non-string items get wrapped in a single Function tuple
583+
argCount++
563584
}
564585
}
565586
}
@@ -600,8 +621,26 @@ func explainInExprWithAlias(sb *strings.Builder, n *ast.InExpr, alias string, in
600621
explainTupleInInList(sb, item.(*ast.Literal), indent+" ", depth+4)
601622
}
602623
} else {
624+
// Check if all items are string literals (large list case)
625+
allStringLiterals := true
603626
for _, item := range n.List {
604-
Node(sb, item, depth+2)
627+
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
628+
allStringLiterals = false
629+
break
630+
}
631+
}
632+
if allStringLiterals {
633+
// Large string list - output as separate children (no tuple wrapper)
634+
for _, item := range n.List {
635+
Node(sb, item, depth+2)
636+
}
637+
} else {
638+
// Wrap non-literal/non-tuple list items in Function tuple
639+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
640+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.List))
641+
for _, item := range n.List {
642+
Node(sb, item, depth+4)
643+
}
605644
}
606645
}
607646
}

‎parser/expression.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,10 @@ func (p *Parser) parseGroupedOrTuple() ast.Expression {
895895
elements := []ast.Expression{first}
896896
for p.currentIs(token.COMMA) {
897897
p.nextToken()
898+
// Handle trailing comma: (1,) should create tuple with single element
899+
if p.currentIs(token.RPAREN) {
900+
break
901+
}
898902
elements = append(elements, p.parseExpression(LOWEST))
899903
}
900904
p.expect(token.RPAREN)

‎parser/testdata/00017_aggregation_uninitialized_memory/metadata.json‎

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

‎parser/testdata/00172_constexprs_in_set/metadata.json‎

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

‎parser/testdata/00604_show_create_database/metadata.json‎

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

‎parser/testdata/00606_quantiles_and_nans/metadata.json‎

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

‎parser/testdata/00840_top_k_weighted/metadata.json‎

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

‎parser/testdata/01021_tuple_parser/metadata.json‎

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

‎parser/testdata/01277_large_tuples/metadata.json‎

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

0 commit comments

Comments
 (0)