-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[branch-51] Revert rewrite for coalesce, nvl and nvl2 simplification
#18567
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ification via CASE Expression (apache#18191)" This reverts commit 22c4214.
This reverts commit e5dcc8c.
This reverts commit 5cc0be5.
This was referenced Nov 9, 2025
37 tasks
zhuqi-lucas
approved these changes
Nov 10, 2025
Member
|
Do we need to regenerate the changelog after the PR? |
Contributor
Author
Yes |
Contributor
Author
|
Thank you @zhuqi-lucas and @xudong963 I'll merge this one in |
Contributor
Author
|
I will make a new PR to update changelog |
Contributor
Author
|
mbutrovich
pushed a commit
that referenced
this pull request
Nov 10, 2025
## Which issue does this PR close? - part of #17558 ## Rationale for this change Per @xudong963 in #18567 (comment), we should update the changelog on `branch-51` to incorporate - #18567 ## What changes are included in this PR? Ran ```shell ./dev/release/generate-changelog.py 50.3.0 branch-51 51.0.0 > dev/changelog/51.0.0.md npx prettier@2.7.1 --write '{datafusion,datafusion-cli,datafusion-examples,dev,docs}/**/*.md' '!datafusion/CHANGELOG.md' README.md CONTRIBUTING.md ``` And checked in the results ## Are these changes tested? By CI ## Are there any user-facing changes? New changelog content
alamb
added a commit
to alamb/datafusion
that referenced
this pull request
Nov 11, 2025
## Which issue does this PR close? - part of apache#17558 ## Rationale for this change Per @xudong963 in apache#18567 (comment), we should update the changelog on `branch-51` to incorporate - apache#18567 ## What changes are included in this PR? Ran ```shell ./dev/release/generate-changelog.py 50.3.0 branch-51 51.0.0 > dev/changelog/51.0.0.md npx prettier@2.7.1 --write '{datafusion,datafusion-cli,datafusion-examples,dev,docs}/**/*.md' '!datafusion/CHANGELOG.md' README.md CONTRIBUTING.md ``` And checked in the results ## Are these changes tested? By CI ## Are there any user-facing changes? New changelog content
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
core
Core DataFusion crate
documentation
Improvements or additions to documentation
functions
Changes to functions implementation
logical-expr
Logical plan and expressions
optimizer
Optimizer rules
sqllogictest
SQL Logic Tests (.slt)
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note this targets the branch-51 release branch
Which issue does this PR close?
51.0.0(Nov 2025) #17558Rationale for this change
coalesceandnvl2to useCASEwhich are faster and more correct (👏 @chenkovsky @kosiew )What changes are included in this PR?
nvla thin wrapper forcoalesce#17991 / ea83c26nvl2Function to Support Lazy Evaluation and Simplification via CASE Expression #18191 / 22c4214Are these changes tested?
Yes I added a new test
Are there any user-facing changes?