sql: improve plan formatting of GROUP BY expressions

erikgrinaker · Jul 15, 2024 · f5a65f0 · f5a65f0
1 parent 5c55e70
commit f5a65f0
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 39 deletions.
diff --git a/src/sql/planner/plan.rs b/src/sql/planner/plan.rs
@@ -342,10 +342,12 @@ impl Node {
         // Format the node.
         match self {
             Self::Aggregate { source, aggregates, group_by } => {
-                write!(f, "Aggregate: {}", aggregates.iter().join(", "))?;
-                if !group_by.is_empty() {
-                    write!(f, " group by {}", group_by.iter().join(", "))?;
-                }
+                let aggregates = group_by
+                    .iter()
+                    .map(|group| format!("group({group})"))
+                    .chain(aggregates.iter().map(|agg| agg.to_string()))
+                    .join(", ");
+                write!(f, "Aggregate: {aggregates}")?;
                 source.format(f, prefix, false, true)?;
             }
             Self::Filter { source, predicate } => {

diff --git a/src/sql/testscripts/optimizer/short_circuit b/src/sql/testscripts/optimizer/short_circuit
@@ -193,10 +193,10 @@ foo, bar
 ---
 Initial:
    Projection: test.id, #1, #2
-   └─ Aggregate: min(id), max(id) group by id
+   └─ Aggregate: group(id), min(id), max(id)
       └─ Scan: test
 Short circuit:
-   Aggregate: min(id), max(id) group by id
+   Aggregate: group(id), min(id), max(id)
    └─ Scan: test
 1, 1, 1
 2, 2, 2

diff --git a/src/sql/testscripts/queries/group_by b/src/sql/testscripts/queries/group_by
@@ -25,13 +25,13 @@ ok
 [plan]> SELECT COUNT(id), MIN(id), MAX(id), SUM(id), AVG(id) FROM test WHERE FALSE GROUP BY id
 ---
 Projection: #1, #2, #3, #4, #5
-└─ Aggregate: count(id), min(id), max(id), sum(id), avg(id) group by id
+└─ Aggregate: group(id), count(id), min(id), max(id), sum(id), avg(id)
    └─ Nothing
 
 # Simple GROUP BY, including NULL group.
 [plan]> SELECT "group", COUNT(*) FROM test GROUP BY "group"
 ---
-Aggregate: count(TRUE) group by group
+Aggregate: group(group), count(TRUE)
 └─ Scan: test
 NULL, 1
 a, 3
@@ -40,7 +40,7 @@ b, 3
 [plan]> SELECT "group", COUNT(*), MIN("bool"), MAX("string"), SUM("int"), AVG("float") \
     FROM test GROUP BY "group"
 ---
-Aggregate: count(TRUE), min(bool), max(string), sum(int), avg(float) group by group
+Aggregate: group(group), count(TRUE), min(bool), max(string), sum(int), avg(float)
 └─ Scan: test
 NULL, 1, NULL, NULL, NULL, NULL
 a, 3, FALSE, AB, 9, NaN
@@ -49,7 +49,7 @@ b, 3, FALSE, 👋, 41, NaN
 # GROUP BY works on booleans.
 [plan]> SELECT "bool", COUNT(*) FROM test GROUP BY "bool"
 ---
-Aggregate: count(TRUE) group by bool
+Aggregate: group(bool), count(TRUE)
 └─ Scan: test
 NULL, 1
 FALSE, 3
@@ -58,7 +58,7 @@ TRUE, 3
 # GROUP BY works on integers.
 [plan]> SELECT "int", COUNT(*) FROM test GROUP BY "int"
 ---
-Aggregate: count(TRUE) group by int
+Aggregate: group(int), count(TRUE)
 └─ Scan: test
 NULL, 1
 -1, 2
@@ -70,7 +70,7 @@ NULL, 1
 # GROUP BY works with floats, including a NAN group and -0.0 and 0.0 being equal.
 [plan]> SELECT "float", COUNT(*) FROM test GROUP BY "float"
 ---
-Aggregate: count(TRUE) group by float
+Aggregate: group(float), count(TRUE)
 └─ Scan: test
 NULL, 1
 0, 2
@@ -81,7 +81,7 @@ NaN, 2
 # GROUP BY works on strings.
 [plan]> SELECT "string", COUNT(*) FROM test GROUP BY "string"
 ---
-Aggregate: count(TRUE) group by string
+Aggregate: group(string), count(TRUE)
 └─ Scan: test
 NULL, 1
 , 2
@@ -93,7 +93,7 @@ abc, 2
 [plan]> SELECT COUNT(*) FROM test GROUP BY "group"
 ---
 Projection: #1
-└─ Aggregate: count(TRUE) group by group
+└─ Aggregate: group(group), count(TRUE)
    └─ Scan: test
 1
 3
@@ -102,7 +102,7 @@ Projection: #1
 # GROUP BY works when there is no aggregate function.
 [plan]> SELECT "group" FROM test GROUP BY "group"
 ---
-Aggregate:  group by group
+Aggregate: group(group)
 └─ Scan: test
 NULL
 a
@@ -115,7 +115,7 @@ Error: invalid input: unknown field g
 
 [plan]> SELECT "group", COUNT(*) FROM test AS t GROUP BY t."group"
 ---
-Aggregate: count(TRUE) group by t.group
+Aggregate: group(t.group), count(TRUE)
 └─ Scan: test as t
 NULL, 1
 a, 3
@@ -141,22 +141,22 @@ Error: invalid input: unknown table unknown
 [plan]> SELECT COUNT(*) FROM test GROUP BY 1
 ---
 Projection: #1
-└─ Aggregate: count(TRUE) group by 1
+└─ Aggregate: group(1), count(TRUE)
    └─ Scan: test
 7
 
 [plan]> SELECT COUNT(*) FROM test GROUP BY id % 2
 ---
 Projection: #1
-└─ Aggregate: count(TRUE) group by id % 2
+└─ Aggregate: group(id % 2), count(TRUE)
    └─ Scan: test
 4
 3
 
 # GROUP BY can use an expression also used in the SELECT.
 [plan]> SELECT id % 2, COUNT(*) FROM test GROUP BY id % 2
 ---
-Aggregate: count(TRUE) group by id % 2
+Aggregate: group(id % 2), count(TRUE)
 └─ Scan: test
 0, 4
 1, 3
@@ -165,7 +165,7 @@ Aggregate: count(TRUE) group by id % 2
 [plan]> SELECT MAX("int") + id % 2 FROM test GROUP BY id
 ---
 Projection: #1 + test.id % 2
-└─ Aggregate: max(int) group by id
+└─ Aggregate: group(id), max(int)
    └─ Scan: test
 NULL
 0
@@ -188,7 +188,7 @@ Error: invalid input: unknown function min
 # GROUP BY works with multiple groups.
 [plan]> SELECT "group", "bool", COUNT(*) FROM test GROUP BY "group", "bool"
 ---
-Aggregate: count(TRUE) group by group, bool
+Aggregate: group(group), group(bool), count(TRUE)
 └─ Scan: test
 NULL, NULL, 1
 a, FALSE, 1
@@ -199,7 +199,7 @@ b, TRUE, 1
 # GROUP BY works with joins.
 [plan]> SELECT t.id % 2, COUNT(*) FROM test t JOIN other o ON t.id % 2 = o.id GROUP BY t.id % 2
 ---
-Aggregate: count(TRUE) group by t.id % 2
+Aggregate: group(t.id % 2), count(TRUE)
 └─ NestedLoopJoin: inner on t.id % 2 = o.id
    ├─ Scan: test as t
    └─ Scan: other as o
@@ -212,7 +212,7 @@ Error: invalid input: field test.group must be used in an aggregate or GROUP BY
 
 [plan]> SELECT * FROM test GROUP BY id, "group", "bool", "int", "float", "string"
 ---
-Aggregate:  group by id, group, bool, int, float, string
+Aggregate: group(id), group(group), group(bool), group(int), group(float), group(string)
 └─ Scan: test
 0, NULL, NULL, NULL, NULL, NULL
 1, a, TRUE, -1, 3.14, 
@@ -225,7 +225,7 @@ Aggregate:  group by id, group, bool, int, float, string
 [plan]> SELECT * FROM test GROUP BY "bool", "int", "float", "string", "group", id
 ---
 Projection: test.id, test.group, test.bool, test.int, test.float, test.string
-└─ Aggregate:  group by bool, int, float, string, group, id
+└─ Aggregate: group(bool), group(int), group(float), group(string), group(group), group(id)
    └─ Scan: test
 0, NULL, NULL, NULL, NULL, NULL
 6, b, FALSE, -1, 0, abc

diff --git a/src/sql/testscripts/queries/having b/src/sql/testscripts/queries/having
@@ -42,15 +42,15 @@ TRUE
 Remap: #0, #1 (dropped: #2)
 └─ Filter: #2 > 10
    └─ Projection: test.group, #1, #1
-      └─ Aggregate: max(int) group by group
+      └─ Aggregate: group(group), max(int)
          └─ Scan: test
 b, 42
 
 [plan]> SELECT "group" FROM test GROUP BY "group" HAVING MAX("int") > 10
 ---
 Remap: #0 (dropped: #1)
 └─ Filter: #1 > 10
-   └─ Aggregate: max(int) group by group
+   └─ Aggregate: group(group), max(int)
       └─ Scan: test
 b
 
@@ -59,15 +59,15 @@ b
 Remap: #0, #1 (dropped: #2, #3)
 └─ Filter: #2 - #3 > 10
    └─ Projection: test.group, #1, #1, #2
-      └─ Aggregate: max(int), min(int) group by group
+      └─ Aggregate: group(group), max(int), min(int)
          └─ Scan: test
 b, 42
 
 # Having works with SELECT aliases.
 [plan]> SELECT "group", MAX("int") AS m FROM test GROUP BY "group" HAVING m > 10
 ---
 Filter: m > 10
-└─ Aggregate: max(int) group by group
+└─ Aggregate: group(group), max(int)
    └─ Scan: test
 b, 42
 
@@ -76,7 +76,7 @@ b, 42
 ---
 Remap: #0, #1 (dropped: #2)
 └─ Filter: #2 > 10
-   └─ Aggregate: count(TRUE), max(int) group by group
+   └─ Aggregate: group(group), count(TRUE), max(int)
       └─ Scan: test
 b, 3
 
@@ -86,7 +86,7 @@ b, 3
 Remap: #0, #1 (dropped: #2, #3)
 └─ Filter: #2 / #3 > 3
    └─ Projection: test.group, #1, #2, #1
-      └─ Aggregate: count(TRUE), max(int) group by group
+      └─ Aggregate: group(group), count(TRUE), max(int)
          └─ Scan: test
 b, 3
 
@@ -97,7 +97,7 @@ b, 3
 Remap: #0 (dropped: #1)
 └─ Filter: 2 - #1 + 1 > 1
    └─ Projection: #1, #0
-      └─ Aggregate: count(TRUE) group by id % 2
+      └─ Aggregate: group(id % 2), count(TRUE)
          └─ Scan: test
 4
 3
@@ -108,7 +108,7 @@ Remap: #0 (dropped: #1)
 Remap: #0 (dropped: #1)
 └─ Filter: test.group = a
    └─ Projection: #1, test.group
-      └─ Aggregate: count(TRUE) group by group
+      └─ Aggregate: group(group), count(TRUE)
          └─ Scan: test
 3
 
@@ -117,7 +117,7 @@ Remap: #0 (dropped: #1)
 Remap: #0 (dropped: #1)
 └─ Filter: group = a
    └─ Projection: #1, group
-      └─ Aggregate: count(TRUE) group by test.group
+      └─ Aggregate: group(test.group), count(TRUE)
          └─ Scan: test
 3
 

diff --git a/src/sql/testscripts/queries/order b/src/sql/testscripts/queries/order
@@ -475,7 +475,7 @@ Order: o.id desc, t.id asc
 Remap: #0, #1 (dropped: #2)
 └─ Order: #2 desc
    └─ Projection: test.bool, #1, #1
-      └─ Aggregate: max(int) group by bool
+      └─ Aggregate: group(bool), max(int)
          └─ Scan: test
 NULL, 1000
 TRUE, 0
@@ -485,7 +485,7 @@ FALSE, -1
 ---
 Remap: #0 (dropped: #1)
 └─ Order: #1 desc
-   └─ Aggregate: max(int) group by bool
+   └─ Aggregate: group(bool), max(int)
       └─ Scan: test
 NULL
 TRUE
@@ -496,7 +496,7 @@ FALSE
 Remap: #0, #1 (dropped: #2, #3)
 └─ Order: #2 - #3 desc
    └─ Projection: test.bool, #1, #1, #2
-      └─ Aggregate: max(int), min(int) group by bool
+      └─ Aggregate: group(bool), max(int), min(int)
          └─ Scan: test
 NULL, 1000
 FALSE, -1
@@ -509,7 +509,7 @@ TRUE, 0
 Remap: #0 (dropped: #1)
 └─ Order: 2 - #1 + 1 > 1 asc
    └─ Projection: #1, #0
-      └─ Aggregate: count(TRUE) group by id % 2
+      └─ Aggregate: group(id % 2), count(TRUE)
          └─ Scan: test
 5
 5
@@ -520,7 +520,7 @@ Remap: #0 (dropped: #1)
 Remap: #0 (dropped: #1)
 └─ Order: test.bool asc
    └─ Projection: #1, test.bool
-      └─ Aggregate: count(TRUE) group by bool
+      └─ Aggregate: group(bool), count(TRUE)
          └─ Scan: test
 8
 1
@@ -531,7 +531,7 @@ Remap: #0 (dropped: #1)
 Remap: #0 (dropped: #1)
 └─ Order: bool asc
    └─ Projection: #1, bool
-      └─ Aggregate: count(TRUE) group by test.bool
+      └─ Aggregate: group(test.bool), count(TRUE)
          └─ Scan: test
 8
 1