Skip to content

Commit 7ff6679

Browse files
committed
Fix large string list handling in IN expressions
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.
1 parent 5effbea commit 7ff6679

File tree

6 files changed

+17
-84
lines changed

6 files changed

+17
-84
lines changed

internal/explain/functions.go

Lines changed: 12 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -419,12 +419,9 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
419419
// Determine if the IN list should be combined into a single tuple literal
420420
// This happens when we have multiple literals of compatible types:
421421
// - All numeric literals/expressions (integers/floats, including unary minus)
422-
// - All string literals (only for small lists, max 10 items)
422+
// - All string literals
423423
// - All tuple literals that contain only primitive literals (recursively)
424424
canBeTupleLiteral := false
425-
// Only combine strings into tuple for small lists (up to 10 items)
426-
// Large string lists are kept as separate children in ClickHouse EXPLAIN AST
427-
const maxStringTupleSize = 10
428425
if n.Query == nil && len(n.List) > 1 {
429426
allNumeric := true
430427
allStrings := true
@@ -457,9 +454,8 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
457454
break
458455
}
459456
}
460-
// For strings, only combine if list is small enough
461457
// For tuples, only combine if all contain primitive literals
462-
canBeTupleLiteral = allNumeric || (allStrings && len(n.List) <= maxStringTupleSize) || (allTuples && allTuplesArePrimitive)
458+
canBeTupleLiteral = allNumeric || allStrings || (allTuples && allTuplesArePrimitive)
463459
}
464460

465461
// Count arguments: expr + list items or subquery
@@ -479,21 +475,8 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
479475
argCount += len(n.List)
480476
}
481477
} else {
482-
// Check if all items are string literals (large list case - no wrapper)
483-
allStringLiterals := true
484-
for _, item := range n.List {
485-
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
486-
allStringLiterals = false
487-
break
488-
}
489-
}
490-
if allStringLiterals {
491-
// Large string list - separate children
492-
argCount += len(n.List)
493-
} else {
494-
// Non-string items get wrapped in a single Function tuple
495-
argCount++
496-
}
478+
// Non-string items get wrapped in a single Function tuple
479+
argCount++
497480
}
498481
}
499482
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, argCount)
@@ -540,26 +523,11 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
540523
explainTupleInInList(sb, item.(*ast.Literal), indent+" ", depth+4)
541524
}
542525
} else {
543-
// Check if all items are string literals (large list case)
544-
allStringLiterals := true
526+
// Wrap non-literal/non-tuple list items in Function tuple
527+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
528+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.List))
545529
for _, item := range n.List {
546-
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
547-
allStringLiterals = false
548-
break
549-
}
550-
}
551-
if allStringLiterals {
552-
// Large string list - output as separate children (no tuple wrapper)
553-
for _, item := range n.List {
554-
Node(sb, item, depth+2)
555-
}
556-
} else {
557-
// Wrap non-literal/non-tuple list items in Function tuple
558-
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
559-
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.List))
560-
for _, item := range n.List {
561-
Node(sb, item, depth+4)
562-
}
530+
Node(sb, item, depth+4)
563531
}
564532
}
565533
}
@@ -705,26 +673,11 @@ func explainInExprWithAlias(sb *strings.Builder, n *ast.InExpr, alias string, in
705673
explainTupleInInList(sb, item.(*ast.Literal), indent+" ", depth+4)
706674
}
707675
} else {
708-
// Check if all items are string literals (large list case)
709-
allStringLiterals := true
676+
// Wrap non-literal/non-tuple list items in Function tuple
677+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
678+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.List))
710679
for _, item := range n.List {
711-
if lit, ok := item.(*ast.Literal); !ok || lit.Type != ast.LiteralString {
712-
allStringLiterals = false
713-
break
714-
}
715-
}
716-
if allStringLiterals {
717-
// Large string list - output as separate children (no tuple wrapper)
718-
for _, item := range n.List {
719-
Node(sb, item, depth+2)
720-
}
721-
} else {
722-
// Wrap non-literal/non-tuple list items in Function tuple
723-
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
724-
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.List))
725-
for _, item := range n.List {
726-
Node(sb, item, depth+4)
727-
}
680+
Node(sb, item, depth+4)
728681
}
729682
}
730683
}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt2": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true
4-
}
5-
}
1+
{}

0 commit comments

Comments
 (0)