Skip to content

Allow aggregation without projection in Unparser #13326

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

blaginin
Copy link
Contributor

@blaginin blaginin commented Nov 9, 2024

Which issue does this PR close?

Closes #12773

Rationale for this change

When parsing a query into a logical plan, aggregation precedes projection. But optimiser can eliminate projection, leaving aggregation as a top node. This is not expected by select_to_sql_recursively because it always handles projection and aggregation together.

What changes are included in this PR?

Added a separate branch if aggregation is the top node.

Are these changes tested?

Yes, added a new test

Are there any user-facing changes?

No

@github-actions github-actions bot added the sql SQL Planner label Nov 9, 2024
@blaginin blaginin force-pushed the fix-aggrefation-without-projection branch from 69be75b to d539fc8 Compare November 9, 2024 17:02
@blaginin blaginin force-pushed the fix-aggrefation-without-projection branch from d539fc8 to 8eece06 Compare November 11, 2024 12:10
@blaginin blaginin marked this pull request as ready for review November 11, 2024 12:18
@blaginin blaginin changed the title Allow aggregation without projection in Unparser Allow aggregation without projection in Unparser Nov 11, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks like a nice improvement to me -- thank you @blaginin

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plan_to_sql produces incorrect SQL for optimised Aggregate plans
2 participants