-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-17849] [SQL] Fix NPE problem when using grouping sets #15416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,10 +298,14 @@ class Analyzer( | |
| case other => Alias(other, other.toString)() | ||
| } | ||
|
|
||
| val nonNullBitmask = x.bitmasks.reduce(_ & _) | ||
| // The left most bit in the bitmasks corresponds to the last expression in groupByAliases | ||
| // with 0 indicating this expression is in the grouping set. The following line of code | ||
| // calculates the bit mask representing the expressions that exist in all the grouping sets. | ||
| val nonNullBitmask = ~ x.bitmasks.reduce(_ | _) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you remove the '~' here, and use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean |
||
|
|
||
| val attrLength = groupByAliases.length | ||
| val expandedAttributes = groupByAliases.zipWithIndex.map { case (a, idx) => | ||
| a.toAttribute.withNullability((nonNullBitmask & 1 << idx) == 0) | ||
| a.toAttribute.withNullability((nonNullBitmask & 1 << (attrLength - idx - 1)) == 0) | ||
| } | ||
|
|
||
| val expand = Expand(x.bitmasks, groupByAliases, expandedAttributes, gid, x.child) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| CREATE TEMPORARY VIEW grouping AS SELECT * FROM VALUES | ||
| ("1", "2", "3", 1), | ||
| ("4", "5", "6", 1), | ||
| ("7", "8", "9", 1) | ||
| as grouping(a, b, c, d); | ||
|
|
||
| -- SPARK-17849: grouping set throws NPE #1 | ||
| SELECT a, b, c, count(d) FROM grouping GROUP BY a, b, c GROUPING SETS (()); | ||
|
|
||
| -- SPARK-17849: grouping set throws NPE #2 | ||
| SELECT a, b, c, count(d) FROM grouping GROUP BY a, b, c GROUPING SETS ((a)); | ||
|
|
||
| -- SPARK-17849: grouping set throws NPE #3 | ||
| SELECT a, b, c, count(d) FROM grouping GROUP BY a, b, c GROUPING SETS ((c)); | ||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| -- Automatically generated by SQLQueryTestSuite | ||
| -- Number of queries: 4 | ||
|
|
||
|
|
||
| -- !query 0 | ||
| CREATE TEMPORARY VIEW grouping AS SELECT * FROM VALUES | ||
| ("1", "2", "3", 1), | ||
| ("4", "5", "6", 1), | ||
| ("7", "8", "9", 1) | ||
| as grouping(a, b, c, d) | ||
| -- !query 0 schema | ||
| struct<> | ||
| -- !query 0 output | ||
|
|
||
|
|
||
|
|
||
| -- !query 1 | ||
| SELECT a, b, c, count(d) FROM grouping GROUP BY a, b, c GROUPING SETS (()) | ||
| -- !query 1 schema | ||
| struct<a:string,b:string,c:string,count(d):bigint> | ||
| -- !query 1 output | ||
| NULL NULL NULL 3 | ||
|
|
||
|
|
||
| -- !query 2 | ||
| SELECT a, b, c, count(d) FROM grouping GROUP BY a, b, c GROUPING SETS ((a)) | ||
| -- !query 2 schema | ||
| struct<a:string,b:string,c:string,count(d):bigint> | ||
| -- !query 2 output | ||
| 1 NULL NULL 1 | ||
| 4 NULL NULL 1 | ||
| 7 NULL NULL 1 | ||
|
|
||
|
|
||
| -- !query 3 | ||
| SELECT a, b, c, count(d) FROM grouping GROUP BY a, b, c GROUPING SETS ((c)) | ||
| -- !query 3 schema | ||
| struct<a:string,b:string,c:string,count(d):bigint> | ||
| -- !query 3 output | ||
| NULL NULL 3 1 | ||
| NULL NULL 6 1 | ||
| NULL NULL 9 1 |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit manipulation magic is hard to follow. This is should be documented better. Could you add a line or two to explain how the bitmasks are structured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hvanhovell @rxin comments are added