Skip to content

[SPARK-51820][SQL][FOLLOWUP][CONNECT] Don't add UnresolvedOrdinal when appending grouping to aggregate expressions #50958

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

Conversation

mihailotim-db
Copy link
Contributor

What changes were proposed in this pull request?

Don't add UnresolvedOrdinal when appending grouping to aggregate expressions in Spark Connect.

Why are the changes needed?

Change is needed to fix a regression caused by #50606 where UnresolvedOrdinal would end up in aggregate expression and propagate all the way to CheckAnalysis where it would throw an error

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added a test case for the correct behavior.

Was this patch authored or co-authored using generative AI tooling?

No.

Copy link
Contributor

@vladimirg-db vladimirg-db left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this.

@mihailotim-db mihailotim-db force-pushed the mihailotim-db/fix_unresolved_ordinal branch from 4c2e4f6 to abc9a0d Compare May 20, 2025 17:30
@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 4b3a653 May 20, 2025
yhuang-db pushed a commit to yhuang-db/spark that referenced this pull request Jun 9, 2025
…hen appending grouping to aggregate expressions

### What changes were proposed in this pull request?
Don't add `UnresolvedOrdinal` when appending grouping to aggregate expressions in Spark Connect.

### Why are the changes needed?
Change is needed to fix a regression caused by apache#50606 where `UnresolvedOrdinal` would end up in aggregate expression and propagate all the way to `CheckAnalysis` where it would throw an error

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Added a test case for the correct behavior.

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes apache#50958 from mihailotim-db/mihailotim-db/fix_unresolved_ordinal.

Authored-by: Mihailo Timotic <mihailo.timotic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants