[SPARK-35026][SQL] Support nested CUBE/ROLLUP/GROUPING SETS in GROUPING SETS #32201

AngersZhuuuu · 2021-04-16T05:25:33Z

What changes were proposed in this pull request?

PG and Oracle both support use CUBE/ROLLUP/GROUPING SETS in GROUPING SETS's grouping set as a sugar syntax.

In this PR, we support it in Spark SQL too

Why are the changes needed?

Keep consistent with PG and oracle

Does this PR introduce any user-facing change?

User can write grouping analytics like

SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS(ROLLUP(a, b));
SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS((a, b), (a), ());
SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS(GROUPING SETS((a, b), (a), ()));

How was this patch tested?

Added Test

…SETS

github-actions · 2021-04-16T05:25:46Z

Test build #754479169 for PR 32201 at commit f18045a.

AngersZhuuuu · 2021-04-16T05:26:22Z

FYI @cloud-fan @maropu

SparkQA · 2021-04-16T06:29:38Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42038/

SparkQA · 2021-04-16T06:34:21Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42038/

SparkQA · 2021-04-16T10:07:51Z

Test build #137463 has finished for PR 32201 at commit f18045a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2021-04-19T02:41:36Z

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

    | expression
    ;

+nestedGroupingSets
+    :  GROUPING SETS '(' nestedGroupingSet (',' nestedGroupingSet)* ')'


What if we have (2 or more)-nested grouping sets? GROUPING SETS(GROUPING SETS(GROUPING SETS(...? I think we need tests for it.

What if we have (2 or more)-nested grouping sets? GROUPING SETS(GROUPING SETS(GROUPING SETS(...? I think we need tests for it.

Good catch..Just tested, PG support nested grouping sets.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

maropu · 2021-04-19T02:47:25Z

sql/core/src/test/resources/sql-tests/inputs/group-analytics.sql

@@ -80,3 +80,8 @@ SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS((a, b), (a), ());
 SELECT a, b, count(1) FROM testData GROUP BY a, CUBE(a, b), GROUPING SETS((a, b), (a), ());
 SELECT a, b, count(1) FROM testData GROUP BY a, CUBE(a, b), ROLLUP(a, b), GROUPING SETS((a, b), (a), ());

+-- Support use CUBE/ROLLUP/GROUPING SETS in GROUPING SETS
+SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS(ROLLUP(a, b));
+SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS((a, b), (a), ());


This test is related to this PR?

This test is related to this PR?

Hmmm what to show

SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS(ROLLUP(a, b));

is equal to

SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS((a, b), (a), ());

But, this test is the same with one in L79?

But, this test is the same with one in L79?

Remove this line

sql/core/src/test/resources/sql-tests/inputs/group-analytics.sql

cloud-fan · 2021-04-19T05:51:24Z

let's update the SQL doc as well

AngersZhuuuu · 2021-04-19T08:16:49Z

let's update the SQL doc as well

Done

maropu · 2021-04-19T08:35:54Z

docs/sql-ref-syntax-qry-select-groupby.md

    `CUBE|ROLLUP` is just a syntax sugar for `GROUPING SETS`, please refer to the sections above for
    how to translate `CUBE|ROLLUP` to `GROUPING SETS`. `group_expression` can be treated as a single-group
-    `GROUPING SETS` under this context. For multiple `GROUPING SETS` in the `GROUP BY` clause, we generate
+    `GROUPING SETS` under this context. `GROUPING SETS` also support nested grouping anlytics, Spark can support
+    use `CUBE|ROLLUP|GROUPING SETS` as `GROUPING SETS`'s `grouping_set`. For multiple `GROUPING SETS` in the `GROUP BY` clause, we generate


GROUPING SETS also support nested grouping anlytics, Spark can support use CUBE|ROLLUP|GROUPING SETS as GROUPING SETS's grouping_set.

=>

GROUPING SETS can have nested GROUPING SETS, CUBE, ROLLUP clauses, e.g. GROUPING SETS(ROLLUP(X, Y), ...)?

How about current？

maropu · 2021-04-19T08:37:53Z

docs/sql-ref-syntax-qry-select-groupby.md

@@ -95,12 +95,13 @@ aggregate_name ( [ DISTINCT ] expression [ , ... ] ) [ FILTER ( WHERE boolean_ex
     (product, warehouse, location), (warehouse), (product), (warehouse, product), ())`.
    The N elements of a `CUBE` specification results in 2^N `GROUPING SETS`.

-* **Mixed Grouping Analytics**
+* **Mixed/Nested Grouping Analytics**


I feel Nested Grouping Analytics looks confusing because GROUPING SETS only accepts the nested cases.

I feel Nested Grouping Analytics looks confusing because GROUPING SETS only accepts the nested cases.

So let's make these two case separated?

Ah, okay. on second thought, **Mixed/Nested Grouping Analytics** looks okay (I couldn't come up with a better title...)

SparkQA · 2021-04-19T09:04:55Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42143/

SparkQA · 2021-04-19T09:11:20Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42143/

SparkQA · 2021-04-19T10:16:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42150/

SparkQA · 2021-04-19T10:16:48Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42150/

SparkQA · 2021-04-19T11:01:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42153/

SparkQA · 2021-04-19T11:01:52Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42153/

SparkQA · 2021-04-19T11:26:34Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42152/

SparkQA · 2021-04-19T11:38:49Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42152/

maropu · 2021-04-19T15:18:32Z

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

+nestedGroupingSet
+    : groupingAnalytics
+    | '(' (expression (',' expression)*)? ')'
+    | expression


nestedGroupingSet : groupingAnalytics | groupingSet ;

?

Done, nice suggestion

maropu · 2021-04-19T15:25:14Z

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

+    | GROUPING SETS '(' nestedGroupingSet (',' nestedGroupingSet)* ')'
+    ;
+
+nestedGroupingSet


How about nestedGroupingSet -> groupingElement?

SparkQA · 2021-04-20T03:54:23Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42192/

SparkQA · 2021-04-20T08:02:38Z

Test build #137664 has finished for PR 32201 at commit 73478ca.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AngersZhuuuu · 2021-04-21T10:13:58Z

ping @cloud-fan

cloud-fan · 2021-04-21T16:37:37Z

docs/sql-ref-syntax-qry-select-groupby.md

@@ -113,7 +116,18 @@ aggregate_name ( [ DISTINCT ] expression [ , ... ] ) [ FILTER ( WHERE boolean_ex
        (warehouse, location),
        (warehouse, size),
        (warehouse))`.
-
+
+    `GROUP BY warehouse, GROUPING SETS((env), GROUPING SETS((product), ()), GROUPING SETS((location, size), (location), (size), ()))`


Can we give an explanation on the behavior instead of examples only? We have a good explaination for the mixed case:

For multiple `GROUPING SETS` in the `GROUP BY` clause, we generate a single `GROUPING SETS` by doing a cross-product of the original `GROUPING SETS`s.

Can we do the same for nested case?

how about current?

docs/sql-ref-syntax-qry-select-groupby.md

cloud-fan · 2021-04-22T04:28:04Z

docs/sql-ref-syntax-qry-select-groupby.md

    `CUBE|ROLLUP` is just a syntax sugar for `GROUPING SETS`, please refer to the sections above for
    how to translate `CUBE|ROLLUP` to `GROUPING SETS`. `group_expression` can be treated as a single-group
    `GROUPING SETS` under this context. For multiple `GROUPING SETS` in the `GROUP BY` clause, we generate
-    a single `GROUPING SETS` by doing a cross-product of the original `GROUPING SETS`s. For example,
+    a single `GROUPING SETS` by doing a cross-product of the original `GROUPING SETS`s. For nested `GROUPING SETS` in the `GROUP BY` clause,


For nested GROUPING SETS in the GROUPING SETS clause

cloud-fan · 2021-04-22T04:29:27Z

docs/sql-ref-syntax-qry-select-groupby.md

-
+
+    `GROUP BY GROUPING SETS(GROUPING SETS(warehouse, product), GROUPING SETS(location, size))` is equivalent to 
+    `GROUP BY  GROUPING SETS((warehouse, product), (location, size))`.


interesting, so nested grouping set is just syntax sugar for specifying a grouping set?

how about

GROUPING SETS(GROUPING SETS((warehouse), (warehouse, product)))

interesting, so nested grouping set is just syntax sugar for specifying a grouping set?

It seems yes, the benefit is we can write CUBE/ROLLUP in GROUPING SETS.
since GROUP BY GROUPING SETS((A, B), (A), (), (A, C)) can write as GROUPING BY(ROLLUP(A, B), (A, C))

SparkQA · 2021-04-22T05:07:19Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42297/

SparkQA · 2021-04-22T05:07:20Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42297/

SparkQA · 2021-04-22T07:28:44Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42312/

cloud-fan · 2021-04-22T07:55:25Z

docs/sql-ref-syntax-qry-select-groupby.md

    `CUBE|ROLLUP` is just a syntax sugar for `GROUPING SETS`, please refer to the sections above for
    how to translate `CUBE|ROLLUP` to `GROUPING SETS`. `group_expression` can be treated as a single-group
    `GROUPING SETS` under this context. For multiple `GROUPING SETS` in the `GROUP BY` clause, we generate
-    a single `GROUPING SETS` by doing a cross-product of the original `GROUPING SETS`s. For example,
+    a single `GROUPING SETS` by doing a cross-product of the original `GROUPING SETS`s. For nested `GROUPING SETS` in the `GROUPING SETS` clause,
+    we generate a top-level `GROUING SETS` by expand selected grouping sets of nested group elements. For example,


..., we simply take its grouping sets and strip it.

It seems your previous example was wrong.

GROUPING SETS(GROUPING SETS(a, b), GROUPING SETS(c, d)) should be GROUPING SETS(a, b, c, d), instead of GROUPING SETS((a, b), (c, d))

It seems your previous example was wrong.

GROUPING SETS(GROUPING SETS(a, b), GROUPING SETS(c, d)) should be GROUPING SETS(a, b, c, d), instead of GROUPING SETS((a, b), (c, d))

Yea， previous example I miss (), current is correct.

cloud-fan · 2021-04-22T07:57:08Z

docs/sql-ref-syntax-qry-select-groupby.md

-
+
+    `GROUP BY GROUPING SETS(GROUPING SETS((warehouse), (warehouse, product)))` is equivalent to 
+    `GROUP BY  GROUPING SETS((warehouse), (warehouse, product))`.


nit: remove the extra space after GROUP BY

cloud-fan · 2021-04-22T07:58:04Z

docs/sql-ref-syntax-qry-select-groupby.md

@@ -113,7 +117,10 @@ aggregate_name ( [ DISTINCT ] expression [ , ... ] ) [ FILTER ( WHERE boolean_ex
        (warehouse, location),
        (warehouse, size),
        (warehouse))`.
-
+
+    `GROUP BY GROUPING SETS(GROUPING SETS((warehouse), (warehouse, product)))` is equivalent to 


let's use 2 nested GROUPING SETS in the example

GROUPING SETS(GROUPING SETS(warehouse), GROUPING SETS((warehouse, product)))

SparkQA · 2021-04-22T09:33:50Z

Test build #137769 has finished for PR 32201 at commit 866a373.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-04-22T10:08:20Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42323/

SparkQA · 2021-04-22T10:08:21Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42323/

SparkQA · 2021-04-22T11:55:51Z

Test build #137783 has finished for PR 32201 at commit cdbe1c7.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AngersZhuuuu · 2021-04-22T12:27:42Z

retest this please

cloud-fan · 2021-04-22T13:08:09Z

thanks, merging to master!

SparkQA · 2021-04-22T13:22:16Z

Test build #137794 has finished for PR 32201 at commit 3b97a8d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-04-22T13:32:38Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42335/

SparkQA · 2021-04-22T13:32:39Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42335/

…NG SETS ### What changes were proposed in this pull request? PG and Oracle both support use CUBE/ROLLUP/GROUPING SETS in GROUPING SETS's grouping set as a sugar syntax. ![image](https://user-images.githubusercontent.com/46485123/114975588-139a1180-9eb7-11eb-8f53-498c1db934e0.png) In this PR, we support it in Spark SQL too ### Why are the changes needed? Keep consistent with PG and oracle ### Does this PR introduce _any_ user-facing change? User can write grouping analytics like ``` SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS(ROLLUP(a, b)); SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS((a, b), (a), ()); SELECT a, b, count(1) FROM testData GROUP BY a, GROUPING SETS(GROUPING SETS((a, b), (a), ())); ``` ### How was this patch tested? Added Test Closes apache#32201 from AngersZhuuuu/SPARK-35026. Authored-by: Angerszhuuuu <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

[SPARK-35026][SQL] Support use CUBE/ROLLUP/GROUPING SETS in GROUPING …

f18045a

…SETS

github-actions bot added the SQL label Apr 16, 2021

maropu reviewed Apr 19, 2021

View reviewed changes

AngersZhuuuu added 3 commits April 19, 2021 15:28

support nested

3e68320

Update AstBuilder.scala

3e6cd44

Update sql-ref-syntax-qry-select-groupby.md

52bbd99

github-actions bot added the DOCS label Apr 19, 2021

remove repeated query

b86873d

maropu reviewed Apr 19, 2021

View reviewed changes

Update sql-ref-syntax-qry-select-groupby.md

9783eeb

Update sql-ref-syntax-qry-select-groupby.md

0b472e8

Update sql-ref-syntax-qry-select-groupby.md

ede77ad

maropu reviewed Apr 19, 2021

View reviewed changes

maropu approved these changes Apr 20, 2021

View reviewed changes

cloud-fan reviewed Apr 21, 2021

View reviewed changes

docs/sql-ref-syntax-qry-select-groupby.md Outdated Show resolved Hide resolved

Update sql-ref-syntax-qry-select-groupby.md

866a373

cloud-fan reviewed Apr 22, 2021

View reviewed changes

Update sql-ref-syntax-qry-select-groupby.md

cdbe1c7

cloud-fan reviewed Apr 22, 2021

View reviewed changes

Update sql-ref-syntax-qry-select-groupby.md

3b97a8d

cloud-fan approved these changes Apr 22, 2021

View reviewed changes

cloud-fan closed this in b22d54a Apr 22, 2021



		`GROUP BY GROUPING SETS(GROUPING SETS(warehouse, product), GROUPING SETS(location, size))` is equivalent to
		`GROUP BY GROUPING SETS((warehouse, product), (location, size))`.



		`GROUP BY GROUPING SETS(GROUPING SETS((warehouse), (warehouse, product)))` is equivalent to
		`GROUP BY GROUPING SETS((warehouse), (warehouse, product))`.

[SPARK-35026][SQL] Support nested CUBE/ROLLUP/GROUPING SETS in GROUPING SETS #32201

[SPARK-35026][SQL] Support nested CUBE/ROLLUP/GROUPING SETS in GROUPING SETS #32201

Conversation

AngersZhuuuu commented Apr 16, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

github-actions bot commented Apr 16, 2021

AngersZhuuuu commented Apr 16, 2021

SparkQA commented Apr 16, 2021

SparkQA commented Apr 16, 2021

SparkQA commented Apr 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Apr 19, 2021

AngersZhuuuu commented Apr 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

SparkQA commented Apr 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Apr 20, 2021

SparkQA commented Apr 20, 2021

AngersZhuuuu commented Apr 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021

AngersZhuuuu commented Apr 22, 2021

cloud-fan commented Apr 22, 2021

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021

SparkQA commented Apr 22, 2021