Skip to content

Commit d1edb0d

Browse files
committed
Allow NULL values in IN clause tuple literal formatting
When formatting IN clauses like `i in (1, 3, NULL)`, the list can now be combined into a compact tuple literal format when it contains a mix of: - Numeric values + NULLs - String values + NULLs Previously, the presence of NULL would force the verbose Function tuple format. Now `in (1, 3, NULL)` correctly outputs `Literal Tuple_(UInt64_1, UInt64_3, NULL)`. Fixes all 16 statements in 01231_operator_null_in and many statements in: - 00441_nulls_in - 00939_test_null_in - 01410_nullable_key_and_index - 01507_transform_null_in - 01558_transform_null_in - 01756_optimize_skip_unused_shards_rewrite_in - 02499_analyzer_aggregate_function_lambda_crash_fix - 03234_evaluate_constant_analyzer - 03393_non_constant_second_argument_for_in - 03578_kv_in_type_casts
1 parent 0a10b2c commit d1edb0d

12 files changed

Lines changed: 41 additions & 90 deletions

File tree

internal/explain/functions.go

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -659,22 +659,28 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
659659

660660
// Determine if the IN list should be combined into a single tuple literal
661661
// This happens when we have multiple literals of compatible types:
662-
// - All numeric literals/expressions (integers/floats, including unary minus)
663-
// - All string literals
662+
// - All numeric literals/expressions (integers/floats, including unary minus) + NULLs
663+
// - All string literals + NULLs
664664
// - All tuple literals that contain only primitive literals (recursively)
665665
canBeTupleLiteral := false
666666
if n.Query == nil && len(n.List) > 1 {
667-
allNumeric := true
668-
allStrings := true
667+
allNumericOrNull := true
668+
allStringsOrNull := true
669669
allTuples := true
670670
allTuplesArePrimitive := true
671+
hasNonNull := false // Need at least one non-null value
671672
for _, item := range n.List {
672673
if lit, ok := item.(*ast.Literal); ok {
674+
if lit.Type == ast.LiteralNull {
675+
// NULL is compatible with both numeric and string lists
676+
continue
677+
}
678+
hasNonNull = true
673679
if lit.Type != ast.LiteralInteger && lit.Type != ast.LiteralFloat {
674-
allNumeric = false
680+
allNumericOrNull = false
675681
}
676682
if lit.Type != ast.LiteralString {
677-
allStrings = false
683+
allStringsOrNull = false
678684
}
679685
if lit.Type != ast.LiteralTuple {
680686
allTuples = false
@@ -686,17 +692,18 @@ func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int)
686692
}
687693
} else if isNumericExpr(item) {
688694
// Unary minus of numeric is still numeric
689-
allStrings = false
695+
hasNonNull = true
696+
allStringsOrNull = false
690697
allTuples = false
691698
} else {
692-
allNumeric = false
693-
allStrings = false
699+
allNumericOrNull = false
700+
allStringsOrNull = false
694701
allTuples = false
695702
break
696703
}
697704
}
698705
// For tuples, only combine if all contain primitive literals
699-
canBeTupleLiteral = allNumeric || allStrings || (allTuples && allTuplesArePrimitive)
706+
canBeTupleLiteral = hasNonNull && (allNumericOrNull || allStringsOrNull || (allTuples && allTuplesArePrimitive))
700707
}
701708

702709
// Count arguments: expr + list items or subquery
@@ -814,17 +821,23 @@ func explainInExprWithAlias(sb *strings.Builder, n *ast.InExpr, alias string, in
814821
const maxStringTupleSizeWithAlias = 10
815822
canBeTupleLiteral := false
816823
if n.Query == nil && len(n.List) > 1 {
817-
allNumeric := true
818-
allStrings := true
824+
allNumericOrNull := true
825+
allStringsOrNull := true
819826
allTuples := true
820827
allTuplesArePrimitive := true
828+
hasNonNull := false // Need at least one non-null value
821829
for _, item := range n.List {
822830
if lit, ok := item.(*ast.Literal); ok {
831+
if lit.Type == ast.LiteralNull {
832+
// NULL is compatible with both numeric and string lists
833+
continue
834+
}
835+
hasNonNull = true
823836
if lit.Type != ast.LiteralInteger && lit.Type != ast.LiteralFloat {
824-
allNumeric = false
837+
allNumericOrNull = false
825838
}
826839
if lit.Type != ast.LiteralString {
827-
allStrings = false
840+
allStringsOrNull = false
828841
}
829842
if lit.Type != ast.LiteralTuple {
830843
allTuples = false
@@ -834,16 +847,17 @@ func explainInExprWithAlias(sb *strings.Builder, n *ast.InExpr, alias string, in
834847
}
835848
}
836849
} else if isNumericExpr(item) {
837-
allStrings = false
850+
hasNonNull = true
851+
allStringsOrNull = false
838852
allTuples = false
839853
} else {
840-
allNumeric = false
841-
allStrings = false
854+
allNumericOrNull = false
855+
allStringsOrNull = false
842856
allTuples = false
843857
break
844858
}
845859
}
846-
canBeTupleLiteral = allNumeric || (allStrings && len(n.List) <= maxStringTupleSizeWithAlias) || (allTuples && allTuplesArePrimitive)
860+
canBeTupleLiteral = hasNonNull && (allNumericOrNull || (allStringsOrNull && len(n.List) <= maxStringTupleSizeWithAlias) || (allTuples && allTuplesArePrimitive))
847861
}
848862

849863
// Count arguments
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
{
22
"explain_todo": {
3-
"stmt1": true,
43
"stmt13": true,
54
"stmt14": true,
65
"stmt15": true,
76
"stmt16": true,
8-
"stmt17": true,
9-
"stmt2": true,
10-
"stmt7": true,
11-
"stmt8": true
7+
"stmt17": true
128
}
139
}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt6": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt10": true,
4-
"stmt12": true,
5-
"stmt13": true,
6-
"stmt15": true,
7-
"stmt17": true,
8-
"stmt19": true,
9-
"stmt20": true,
10-
"stmt22": true,
11-
"stmt23": true,
12-
"stmt25": true,
13-
"stmt26": true,
14-
"stmt28": true,
15-
"stmt4": true,
16-
"stmt6": true,
17-
"stmt7": true,
18-
"stmt9": true
19-
}
20-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt26": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt4": true,
4-
"stmt6": true,
5-
"stmt9": true
6-
}
7-
}
1+
{}
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt15": true,
4-
"stmt16": true,
5-
"stmt17": true,
6-
"stmt18": true,
7-
"stmt19": true,
8-
"stmt20": true,
9-
"stmt21": true,
10-
"stmt5": true,
11-
"stmt6": true,
12-
"stmt9": true
3+
"stmt21": true
134
}
145
}

parser/testdata/01756_optimize_skip_unused_shards_rewrite_in/metadata.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
"explain_todo": {
33
"stmt45": true,
44
"stmt46": true,
5-
"stmt53": true,
65
"stmt56": true,
76
"stmt57": true
87
}
Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt2": true,
4-
"stmt3": true
5-
}
6-
}
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)