Add GROUPING SETS support for EXPLAIN AST output

claude · claude · commit a899fb69cd7b · 2025-12-31T07:35:24.000Z
Each grouping set element in GROUPING SETS clause needs to be wrapped
in an ExpressionList in the EXPLAIN output. Added GroupingSets flag to
SelectQuery AST to track when GROUPING SETS is used, and updated the
explain code to output the wrapper when needed.

Fixes 66 statements across 25 tests.
diff --git a/ast/ast.go b/ast/ast.go
@@ -64,6 +64,7 @@ type SelectQuery struct {
 	PreWhere    Expression            `json:"prewhere,omitempty"`
 	Where       Expression            `json:"where,omitempty"`
 	GroupBy     []Expression          `json:"group_by,omitempty"`
+	GroupingSets bool                 `json:"grouping_sets,omitempty"` // true if GROUP BY uses GROUPING SETS
 	WithRollup  bool                  `json:"with_rollup,omitempty"`
 	WithCube    bool                  `json:"with_cube,omitempty"`
 	WithTotals  bool                  `json:"with_totals,omitempty"`
diff --git a/internal/explain/select.go b/internal/explain/select.go
@@ -100,7 +100,13 @@ func explainSelectQuery(sb *strings.Builder, n *ast.SelectQuery, indent string,
 	if len(n.GroupBy) > 0 {
 		fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.GroupBy))
 		for _, g := range n.GroupBy {
-			Node(sb, g, depth+2)
+			if n.GroupingSets {
+				// Each grouping set is wrapped in an ExpressionList
+				fmt.Fprintf(sb, "%s  ExpressionList (children 1)\n", indent)
+				Node(sb, g, depth+3)
+			} else {
+				Node(sb, g, depth+2)
+			}
 		}
 	}
 	// HAVING
diff --git a/parser/parser.go b/parser/parser.go
@@ -534,6 +534,7 @@ func (p *Parser) parseSelect() *ast.SelectQuery {
 			p.nextToken() // skip GROUPING
 			p.nextToken() // skip SETS
 			sel.GroupBy = p.parseGroupingSets()
+			sel.GroupingSets = true
 		} else if p.currentIs(token.ROLLUP) && p.peekIs(token.LPAREN) {
 			// ROLLUP(a, b, c)
 			p.nextToken() // skip ROLLUP
diff --git a/parser/testdata/01883_grouping_sets_crash/metadata.json b/parser/testdata/01883_grouping_sets_crash/metadata.json
@@ -3,7 +3,6 @@
     "stmt10": true,
     "stmt11": true,
     "stmt12": true,
-    "stmt13": true,
     "stmt14": true,
     "stmt4": true,
     "stmt6": true,
diff --git a/parser/testdata/01883_with_grouping_sets/metadata.json b/parser/testdata/01883_with_grouping_sets/metadata.json
@@ -3,9 +3,6 @@
     "stmt10": true,
     "stmt11": true,
     "stmt12": true,
-    "stmt14": true,
-    "stmt15": true,
-    "stmt4": true,
     "stmt6": true,
     "stmt7": true,
     "stmt8": true,
diff --git a/parser/testdata/02293_grouping_function/metadata.json b/parser/testdata/02293_grouping_function/metadata.json
@@ -1,13 +1,5 @@
 {
   "explain_todo": {
-    "stmt10": true,
-    "stmt2": true,
-    "stmt3": true,
-    "stmt4": true,
-    "stmt5": true,
-    "stmt6": true,
-    "stmt7": true,
-    "stmt8": true,
-    "stmt9": true
+    "stmt7": true
   }
 }
diff --git a/parser/testdata/02343_group_by_use_nulls/metadata.json b/parser/testdata/02343_group_by_use_nulls/metadata.json
@@ -1,7 +1 @@
-{
-  "explain_todo": {
-    "stmt10": true,
-    "stmt6": true,
-    "stmt7": true
-  }
-}
+{}
diff --git a/parser/testdata/02343_group_by_use_nulls_distributed/metadata.json b/parser/testdata/02343_group_by_use_nulls_distributed/metadata.json
@@ -1,6 +1 @@
-{
-  "explain_todo": {
-    "stmt6": true,
-    "stmt7": true
-  }
-}
+{}
diff --git a/parser/testdata/02481_analyzer_optimize_grouping_sets_keys/metadata.json b/parser/testdata/02481_analyzer_optimize_grouping_sets_keys/metadata.json
@@ -1,6 +1,5 @@
 {
   "explain_todo": {
-    "stmt3": true,
     "stmt4": true,
     "stmt5": true,
     "stmt6": true
diff --git a/parser/testdata/02521_grouping_sets_plus_memory_efficient_aggr/metadata.json b/parser/testdata/02521_grouping_sets_plus_memory_efficient_aggr/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt2": true
-  }
-}
+{}
diff --git a/parser/testdata/02534_analyzer_grouping_function/metadata.json b/parser/testdata/02534_analyzer_grouping_function/metadata.json
@@ -1,8 +1 @@
-{
-  "explain_todo": {
-    "stmt11": true,
-    "stmt12": true,
-    "stmt13": true,
-    "stmt14": true
-  }
-}
+{}
diff --git a/parser/testdata/02535_analyzer_group_by_use_nulls/metadata.json b/parser/testdata/02535_analyzer_group_by_use_nulls/metadata.json
@@ -1,8 +1 @@
-{
-  "explain_todo": {
-    "stmt12": true,
-    "stmt16": true,
-    "stmt8": true,
-    "stmt9": true
-  }
-}
+{}
diff --git a/parser/testdata/02541_analyzer_grouping_sets_crash_fix/metadata.json b/parser/testdata/02541_analyzer_grouping_sets_crash_fix/metadata.json
@@ -1,6 +1 @@
-{
-  "explain_todo": {
-    "stmt2": true,
-    "stmt3": true
-  }
-}
+{}
diff --git a/parser/testdata/02579_fill_empty_chunk/metadata.json b/parser/testdata/02579_fill_empty_chunk/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt3": true
-  }
-}
+{}
diff --git a/parser/testdata/02579_fill_empty_chunk_analyzer/metadata.json b/parser/testdata/02579_fill_empty_chunk_analyzer/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt3": true
-  }
-}
+{}
diff --git a/parser/testdata/02835_fuzz_remove_redundant_sorting/metadata.json b/parser/testdata/02835_fuzz_remove_redundant_sorting/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt4": true
-  }
-}
+{}
diff --git a/parser/testdata/02917_transform_tsan/metadata.json b/parser/testdata/02917_transform_tsan/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt1": true
-  }
-}
+{}
diff --git a/parser/testdata/02932_group_by_null_fuzzer/metadata.json b/parser/testdata/02932_group_by_null_fuzzer/metadata.json
@@ -1,6 +1 @@
-{
-  "explain_todo": {
-    "stmt2": true,
-    "stmt4": true
-  }
-}
+{}
diff --git a/parser/testdata/02962_analyzer_resolve_group_by_on_shards/metadata.json b/parser/testdata/02962_analyzer_resolve_group_by_on_shards/metadata.json
@@ -1,6 +1 @@
-{
-  "explain_todo": {
-    "stmt1": true,
-    "stmt2": true
-  }
-}
+{}
diff --git a/parser/testdata/02992_analyzer_group_by_const/metadata.json b/parser/testdata/02992_analyzer_group_by_const/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt7": true
-  }
-}
+{}
diff --git a/parser/testdata/03013_group_by_use_nulls_with_materialize_and_analyzer/metadata.json b/parser/testdata/03013_group_by_use_nulls_with_materialize_and_analyzer/metadata.json
@@ -1,9 +1 @@
-{
-  "explain_todo": {
-    "stmt5": true,
-    "stmt6": true,
-    "stmt7": true,
-    "stmt8": true,
-    "stmt9": true
-  }
-}
+{}
diff --git a/parser/testdata/03023_group_by_use_nulls_analyzer_crashes/metadata.json b/parser/testdata/03023_group_by_use_nulls_analyzer_crashes/metadata.json
@@ -1,9 +1,5 @@
 {
   "explain_todo": {
-    "stmt11": true,
-    "stmt14": true,
-    "stmt15": true,
-    "stmt4": true,
-    "stmt5": true
+    "stmt11": true
   }
 }
diff --git a/parser/testdata/03150_grouping_sets_use_nulls_pushdown/metadata.json b/parser/testdata/03150_grouping_sets_use_nulls_pushdown/metadata.json
@@ -5,7 +5,6 @@
     "stmt5": true,
     "stmt6": true,
     "stmt7": true,
-    "stmt8": true,
-    "stmt9": true
+    "stmt8": true
   }
 }
diff --git a/parser/testdata/03231_dynamic_variant_in_order_by_group_by/metadata.json b/parser/testdata/03231_dynamic_variant_in_order_by_group_by/metadata.json
@@ -1,14 +1,6 @@
 {
   "explain_todo": {
-    "stmt101": true,
-    "stmt115": true,
-    "stmt126": true,
     "stmt17": true,
-    "stmt37": true,
-    "stmt48": true,
-    "stmt62": true,
-    "stmt73": true,
-    "stmt8": true,
-    "stmt90": true
+    "stmt8": true
   }
 }
diff --git a/parser/testdata/03259_grouping_sets_aliases/metadata.json b/parser/testdata/03259_grouping_sets_aliases/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt4": true
-  }
-}
+{}
diff --git a/parser/testdata/03358_block_structure_match/metadata.json b/parser/testdata/03358_block_structure_match/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt4": true
-  }
-}
+{}
diff --git a/parser/testdata/03392_crash_group_by_use_nulls/metadata.json b/parser/testdata/03392_crash_group_by_use_nulls/metadata.json
@@ -1,7 +1,5 @@
 {
   "explain_todo": {
-    "stmt1": true,
-    "stmt2": true,
-    "stmt3": true
+    "stmt1": true
   }
 }
diff --git a/parser/testdata/03749_table_function_argument_asterisk/metadata.json b/parser/testdata/03749_table_function_argument_asterisk/metadata.json
@@ -1,5 +1 @@
-{
-  "explain_todo": {
-    "stmt5": true
-  }
-}
+{}

Original file line number	Diff line number	Diff line change
`@@ -1,13 +1,5 @@`
`1`	`1`	`{`
`2`	`2`	`"explain_todo": {`
`3`		`- "stmt10": true,`
`4`		`- "stmt2": true,`
`5`		`- "stmt3": true,`
`6`		`- "stmt4": true,`
`7`		`- "stmt5": true,`
`8`		`- "stmt6": true,`
`9`		`- "stmt7": true,`
`10`		`- "stmt8": true,`
`11`		`- "stmt9": true`
	`3`	`+ "stmt7": true`
`12`	`4`	`}`
`13`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,5 @@`
`1`	`1`	`{`
`2`	`2`	`"explain_todo": {`
`3`		`- "stmt3": true,`
`4`	`3`	`"stmt4": true,`
`5`	`4`	`"stmt5": true,`
`6`	`5`	`"stmt6": true`
-Original file line number
+Diff line change
@@ @@ -1,5 +1 @@ @@
 -{
 -  "explain_todo": {
 -    "stmt2": true
 -  }
 -}
 +{}