You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "mihailoale-db (via GitHub)" <gi...@apache.org> on 2025/12/30 11:50:13 UTC
[PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
mihailoale-db opened a new pull request, #53644:
URL: https://github.com/apache/spark/pull/53644
### What changes were proposed in this pull request?
In this PR I propose to trim aliases from grouping and aggregate expressions before handling grouping analytics. This is needed for the following query:
```
SELECT col1 AS k2 FROM values(1) GROUP BY CUBE(k2)
```
Here we have `col1` in the single-pass whereas in the fixed-point we have `col1 AS k2` before constructing an `Aggregate` in `ResolveGroupingAnalytics`. Change removes the `AS k2` part and keeps the compatibility between single-pass and fixed-point analyzers without changing outputs (analyzed plans are different, only names).
### Why are the changes needed?
To keep the compatibility between fixed-point and single-pass analyzers.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Changed tests.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #53644: [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics
URL: https://github.com/apache/spark/pull/53644
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on PR #53644:
URL: https://github.com/apache/spark/pull/53644#issuecomment-3699610800
@mihailotim-db Done! @cloud-fan PTAL when you find time. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #53644:
URL: https://github.com/apache/spark/pull/53644#issuecomment-3707104200
thanks, merging to master!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #53644:
URL: https://github.com/apache/spark/pull/53644#discussion_r2653262908
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -755,6 +755,7 @@ class Analyzer(
groupByExprs: Seq[Expression],
aggregationExprs: Seq[NamedExpression],
child: LogicalPlan): LogicalPlan = {
+ val aggregationExpressionsWithoutAliases = aggregationExprs.map(trimNonTopLevelAliases)
Review Comment:
The examples in the PR description seems unrelated, as they are not non-top-level aliases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #53644:
URL: https://github.com/apache/spark/pull/53644#discussion_r2655349534
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -755,6 +755,7 @@ class Analyzer(
groupByExprs: Seq[Expression],
aggregationExprs: Seq[NamedExpression],
child: LogicalPlan): LogicalPlan = {
+ val aggregationExpressionsWithoutAliases = aggregationExprs.map(trimNonTopLevelAliases)
Review Comment:
the name seems too long, how about `aggExprsNoAlias`
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/grouping.scala:
##########
@@ -267,24 +267,26 @@ object GroupingID {
}
}
-object GroupingAnalytics {
+object GroupingAnalytics extends AliasHelper {
def unapply(exprs: Seq[Expression])
: Option[(Seq[Seq[Expression]], Seq[Expression])] = {
- if (!exprs.exists(_.isInstanceOf[BaseGroupingSets])) {
+ val expressionsWithoutAliases = exprs.map(trimAliases)
Review Comment:
ditto, `exprsNoAlias`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on code in PR #53644:
URL: https://github.com/apache/spark/pull/53644#discussion_r2653327034
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -755,6 +755,7 @@ class Analyzer(
groupByExprs: Seq[Expression],
aggregationExprs: Seq[NamedExpression],
child: LogicalPlan): LogicalPlan = {
+ val aggregationExpressionsWithoutAliases = aggregationExprs.map(trimNonTopLevelAliases)
Review Comment:
The example is related to the change in the `object GroupingAnalytics`. This one we did to keep this and other changes in sync e.g. https://github.com/apache/spark/pull/51591/files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #53644:
URL: https://github.com/apache/spark/pull/53644#issuecomment-3699119079
## JIRA Issue Information
=== Improvement SPARK-54871 ===
Summary: Trim aliases from grouping and aggregate expressions before handling grouping analytics
Assignee: None
Status: Open
Affected: ["4.1.0"]
---
*This comment was automatically generated by GitHub Actions*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54871] [SQL] Trim aliases from grouping and aggregate expressions before handling grouping analytics [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on code in PR #53644:
URL: https://github.com/apache/spark/pull/53644#discussion_r2655714248
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -755,6 +755,7 @@ class Analyzer(
groupByExprs: Seq[Expression],
aggregationExprs: Seq[NamedExpression],
child: LogicalPlan): LogicalPlan = {
+ val aggregationExpressionsWithoutAliases = aggregationExprs.map(trimNonTopLevelAliases)
Review Comment:
went with `aggregationExprsNoAlias`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org