Skip to content
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

if none columns in window expr are needed, remove the window exprs #2634

Merged
merged 2 commits into from
May 29, 2022
Merged

if none columns in window expr are needed, remove the window exprs #2634

merged 2 commits into from
May 29, 2022

Conversation

HuSen8891
Copy link
Contributor

Which issue does this PR close?

Closes #2542

Rationale for this change

if the columns in window exprs are not referenced by operator, we can remove the window exprs.

What changes are included in this PR?

remove the window exprs in projection_push_down.rs

@github-actions github-actions bot added core Core DataFusion crate datafusion Changes in the datafusion crate optimizer Optimizer rules labels May 28, 2022
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.

Thank you @AssHero -- can you please add a test case to this PR showing that it fixes the issue reported on #2542?

Perhaps you can add the reproducer from @mateuszkj to https://github.com/apache/arrow-datafusion/blob/master/datafusion/core/tests/sql/window.rs?

@HuSen8891
Copy link
Contributor Author

Thank you @AssHero -- can you please add a test case to this PR showing that it fixes the issue reported on #2542?

Perhaps you can add the reproducer from @mateuszkj to https://github.com/apache/arrow-datafusion/blob/master/datafusion/core/tests/sql/window.rs?

yes, i can add a test case.

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 good to me -- thank you @AssHero

@alamb alamb merged commit f9dddad into apache:master May 29, 2022
@HuSen8891
Copy link
Contributor Author

Looks good to me -- thank you @AssHero

My pleasure

@HuSen8891 HuSen8891 deleted the window_func branch May 30, 2022 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate datafusion Changes in the datafusion crate optimizer Optimizer rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unused Window functions experssion is wrongly removed from LogicalPlan during optimalization
2 participants