[SPARK-57775][SQL][4.2] Return one row for GROUP BY GROUPING SETS (()) over empty input#57134
[SPARK-57775][SQL][4.2] Return one row for GROUP BY GROUPING SETS (()) over empty input#57134uros-b wants to merge 2 commits into
Conversation
Co-authored-by: Isaac
dongjoon-hyun
left a comment
There was a problem hiding this comment.
When we introduce a breaking change forcefully like this, we need to update the SQL migration guide too, @uros-b .
cc @huaxingao since this is a correctness issue
uros-b
left a comment
There was a problem hiding this comment.
Thank you for the review @dongjoon-hyun, PTAL again.
Adding @huaxingao too, for 4.2 backport review.
Also, waiting for CI.
|
also cc @MaxGekk @cloud-fan who reviewed the original PR (#56891) |
| - Since Spark 4.2, when a SQL UDF has a parameter whose name matches a parameterless built-in function (`current_user`, `current_date`, `current_time`, `current_timestamp`, `user`, `session_user`, `grouping__id`), a bare reference to that name in the function body resolves to the built-in function instead of the parameter, matching the documented name resolution rules. Rename the parameter to avoid the collision, or set `spark.sql.legacy.allowUdfParameterToShadowParameterlessFunction` to `true` to restore the previous behavior. | ||
| - Since Spark 4.2, `SET CATALOG <name>` resolves a bare (unquoted) name as a session variable first, using the variable's value as the catalog name when such a variable exists, and otherwise treats the name as a literal catalog name. Use a string literal (`SET CATALOG 'name'`) to force it to be interpreted literally. | ||
| - Since Spark 4.2, when an error occurs while collecting observed metrics, `Observation.get` raises the underlying exception (for example, `SparkRuntimeException` in Scala or `PySparkException` in Python) instead of silently returning an empty result. Add error handling if your code relied on receiving an empty result on failure. | ||
| - Since Spark 4.2, `GROUP BY GROUPING SETS (())` (and the equivalent empty `GROUP BY CUBE()` / `GROUP BY ROLLUP()`) is treated as a grand total and returns one row over empty input, matching an aggregation with no `GROUP BY` clause; previously it returned no rows. To restore the previous behavior, set `spark.sql.analyzer.lowerEmptyGroupingSetToGlobalAggregate.enabled` to `false`. |
There was a problem hiding this comment.
if we decided to backport to 4.2, we should remove the migration guide in 4.2 to 4.3 section, as we don't need to duplicate it in two versions. (with a followup master branch pr)
| val LOWER_EMPTY_GROUPING_SET_TO_GLOBAL_AGGREGATE = | ||
| buildConf("spark.sql.analyzer.lowerEmptyGroupingSetToGlobalAggregate.enabled") | ||
| .internal() | ||
| .version("4.2.0") |
There was a problem hiding this comment.
ditto, need a followup master branch to correct the conf version
What changes were proposed in this pull request?
Note: this is a 4.2 backport of #56891.
Lower the single-empty-grouping-set case to a global
Aggregate(no grouping expressions, noExpand) inGroupingAnalyticsTransformer, so it returns one (grand total) row over empty input, matching theGROUP BY-less form and the SQL standard.grouping_id()folds to the constant0, andgrouping()/grouping_id()inHAVING/ORDER BYresolve against that constant. The fix lands in both the legacy fixed-point analyzer and the single-pass resolver, which shareGroupingAnalyticsTransformer.Why are the changes needed?
GROUP BY GROUPING SETS (())is a grand total, semantically identical to an aggregation with noGROUP BYclause. It was lowered to a groupedAggregateover anExpand(grouping byspark_grouping_id), so over empty input it returned zero rows instead of one. The same defect affected the equivalent emptyGROUP BY CUBE()andGROUP BY ROLLUP(), which also lower to a single empty grouping set.Does this PR introduce any user-facing change?
Yes, the new (correct) behavior is gated by an internal SQL config,
spark.sql.analyzer.lowerEmptyGroupingSetToGlobalAggregate.enabled(default true). When set to false, lowering reverts to the legacyExpand-based form (zero rows over empty input). The flag gates all three decision points (the transformer lowering and thegrouping_id()resolution in each analyzer) so the off state reproduces pre-fix behavior identically in both analyzers.How was this patch tested?
Tested via golden cases in
grouping_set.sql(empty-input grand total,grouping_id()in SELECT/HAVING/ORDER BY,grouping()rejection, non-empty input), flag-off coverage ingrouping_set_grand_total_disabled.sql, the regeneratedgroup-analyticsgolden file, andResolveGroupingAnalyticsSuite.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Opus 4.8