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

Fix: regularize order bys when consuming from substrait #14125

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

gabotechs
Copy link
Contributor

@gabotechs gabotechs commented Jan 14, 2025

Which issue does this PR close?

No issue added, let me know if one is needed

Rationale for this change

Fixing an issue with the current substrait consumer

What changes are included in this PR?

Using built-in method for regularizing order by statements based on window frames in the substrait consumer

Are these changes tested?

Yes

Are there any user-facing changes?

Anyone that is passing substrait plans directly to DataFusion should be able to pass window function nodes with no order bys and with a "RANGE" window frame unit.

@github-actions github-actions bot added the substrait Changes to the substrait crate label Jan 14, 2025
@gabotechs gabotechs marked this pull request as ready for review January 14, 2025 18:43
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 @gabotechs

The code looks good to me and I verified that this test fails without the code changes in this PR.

FYI @Blizzara and @vbarua

assertion `left == right` failed
  left: 0
 right: 1

@alamb alamb merged commit 9403448 into apache:main Jan 16, 2025
25 checks passed
@alamb
Copy link
Contributor

alamb commented Jan 16, 2025

Thanks again @gabotechs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
substrait Changes to the substrait crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants