[SPARK-54961] [SQL] Introduce GroupingAnalyticsTransformer#53729
[SPARK-54961] [SQL] Introduce GroupingAnalyticsTransformer#53729mihailoale-db wants to merge 1 commit intoapache:masterfrom
GroupingAnalyticsTransformer#53729Conversation
JIRA Issue Information=== Improvement SPARK-54961 === This comment was automatically generated by GitHub Actions |
|
@vladimirg-db @mihailotim-db Please review. Besides what's mentioned in the PR desc, we have few expression tree traversals e.g. in |
mihailoale-db
force-pushed
the
gatransformer
branch
from
6bca5b9 to
4e05de0
Compare
mihailoale-db
force-pushed
the
gatransformer
branch
2 times, most recently
from
84cd9b1 to
362d563
Compare
| } | ||
|
|
||
| val groupingSetsAttributes = selectedGroupByExpressions.map { groupingSetExprs => | ||
| groupingSetExprs.map { expression => |
There was a problem hiding this comment.
The operation here is very slow. We iterate over groupingSetExprs, inside we iterate over groupByAliases, and every iteration does a semanticEquals tree traversal with a heavy comparison operator. Let's merge this as-is for the purposes of the refactor, but we should followup with a transition to SemanticComparator maybe? wdyt?
There was a problem hiding this comment.
Agreed. I'll followup.
vladimirg-db
left a comment
vladimirg-db
left a comment
There was a problem hiding this comment.
LGTM for the refactor, but let's consider optimization followups
mihailoale-db
force-pushed
the
gatransformer
branch
from
362d563 to
b1554e9
Compare
mihailoale-db
force-pushed
the
gatransformer
branch
from
b1554e9 to
05efae3
Compare
|
thanks, merging to master! |
3 participants
What changes were proposed in this pull request?
In this PR I propose to introduce
GroupingAnalyticsTransformerso we could reuse grouping analytics related code in the single-pass implementation.Besides the refactor we update nullabilities of
Expandoperator in place instead waiting onUpdateAttributeNullabilityto trigger.Why are the changes needed?
To reuse grouping analytics related code in the single-pass implementation.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing tests.
Was this patch authored or co-authored using generative AI tooling?
No.