Skip to content

Fix WindowFrame::new with order_by #16537

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

Merged
merged 2 commits into from
Jun 25, 2025
Merged

Conversation

findepi
Copy link
Member

@findepi findepi commented Jun 24, 2025

Which issue does this PR close?

  • Closes #.

Rationale for this change

Before the change, the frame constructed with
WindowFrame::new(Some(true)) would not be fully functional, because
of the bound being of unexpected type.

What changes are included in this PR?

Change in WindowFrame::new

Are these changes tested?

with unit tests

Are there any user-facing changes?

yes

Putting expected data second allows for natural inlining of expected
values.
@github-actions github-actions bot added logical-expr Logical plan and expressions core Core DataFusion crate labels Jun 24, 2025
@findepi findepi force-pushed the findepi/window branch 2 times, most recently from be53501 to 4ad54f8 Compare June 24, 2025 16:04
Before the change, the frame constructed with
`WindowFrame::new(Some(true))` would not be fully functional, because
of the bound being of unexpected type.
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 @findepi -- this makes change makes sense to me

@findepi findepi merged commit 20a723b into apache:main Jun 25, 2025
27 checks passed
@findepi findepi deleted the findepi/window branch June 25, 2025 07:51
findepi added a commit to sdf-labs/arrow-datafusion that referenced this pull request Jun 25, 2025
* Fix WindowFrame::new with order_by

Before the change, the frame constructed with
`WindowFrame::new(Some(true))` would not be fully functional, because
of the bound being of unexpected type.

* Change assert helper param order

Putting expected data second allows for natural inlining of expected
values.

(cherry picked from commit 20a723b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants