Skip to content

[branch-51] Revert rewrite for coalesce, nvl and nvl2 simplification#18567

Merged
alamb merged 6 commits intoapache:branch-51from
alamb:alamb/revert-rewrites
Nov 10, 2025
Merged

[branch-51] Revert rewrite for coalesce, nvl and nvl2 simplification#18567
alamb merged 6 commits intoapache:branch-51from
alamb:alamb/revert-rewrites

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Nov 9, 2025 •
edited
Loading

Note this targets the branch-51 release branch

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

  1. Revert fix: lazy evaluation for coalesce #17357 / e5dcc8c
  2. Revert #17982 Make nvl a thin wrapper for coalesce #17991 / ea83c26
  3. Revert Refactor nvl2 Function to Support Lazy Evaluation and Simplification via CASE Expression #18191 / 22c4214
  4. Cherry-pick 6202254, a test that reproduces the schema mismatch issue (from Avoid double optimizing in tpchds_planning tests to avoid masking errors #18536)
  5. Cherry-pick 735cacf, a fix for the benchmarks that regressed due to the revert (from Revert "Disable failing benchmark query (#17809)" #17833)
  6. Update datafusion-testing (see separate PR here) for extended tests (see [branch-51] Update for revert of coalesce / nvl / nvl2 datafusion-testing#15)

Are these changes tested?

Yes I added a new test

Are there any user-facing changes?

@github-actions github-actions bot added documentation Improvements or additions to documentation sql SQL Planner development-process Related to development process of DataFusion logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) substrait Changes to the substrait crate catalog Related to the catalog crate common Related to common crate execution Related to the execution crate proto Related to proto crate functions Changes to functions implementation datasource Changes to the datasource crate ffi Changes to the ffi crate physical-plan Changes to the physical-plan crate spark labels Nov 9, 2025
@alamb alamb changed the base branch from branch-50 to branch-51 November 9, 2025 11:52
This was referenced Nov 9, 2025
Closed
Closed
Closed
Closed
Closed
@alamb alamb closed this Nov 9, 2025
@alamb alamb reopened this Nov 9, 2025
@alamb alamb mentioned this pull request Nov 9, 2025
Merged
@github-actions github-actions bot removed sql SQL Planner development-process Related to development process of DataFusion physical-expr Changes to the physical-expr crates substrait Changes to the substrait crate catalog Related to the catalog crate common Related to common crate execution Related to the execution crate proto Related to proto crate datasource Changes to the datasource crate ffi Changes to the ffi crate physical-plan Changes to the physical-plan crate spark labels Nov 9, 2025
@alamb alamb marked this pull request as ready for review November 9, 2025 12:50
@alamb alamb mentioned this pull request Nov 9, 2025
Closed
37 tasks
@xudong963
Copy link
Member

xudong963 commented Nov 10, 2025

Do we need to regenerate the changelog after the PR?

@alamb
Copy link
Contributor Author

alamb commented Nov 10, 2025

Do we need to regenerate the changelog after the PR?

Yes

@alamb
Copy link
Contributor Author

alamb commented Nov 10, 2025

Thank you @zhuqi-lucas and @xudong963 I'll merge this one in

@alamb alamb merged commit 2677c27 into apache:branch-51 Nov 10, 2025
56 of 59 checks passed
@alamb
Copy link
Contributor Author

alamb commented Nov 10, 2025

I will make a new PR to update changelog

@alamb alamb deleted the alamb/revert-rewrites branch November 10, 2025 11:56
@alamb alamb mentioned this pull request Nov 10, 2025
Merged
@alamb
Copy link
Contributor Author

alamb commented Nov 10, 2025 •
edited
Loading

I will make a new PR to update changelog

mbutrovich pushed a commit that referenced this pull request Nov 10, 2025
@alamb
[branch-51] Update Changelog (#18592)
49e347b 
## 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
@alamb
[branch-51] Update Changelog (apache#18592)
a5c0a12 
## 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

Reviewers

@zhuqi-lucas zhuqi-lucas zhuqi-lucas approved these changes

Assignees

No one assigned

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)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alamb @xudong963 @zhuqi-lucas