Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Sep 29, 2025

This reverts commit 5cc0be5.

Which issue does this PR close?

Rationale for this change

I disabled a failing test while we fix it for real (see #17813). I do not want to forget about the test

What changes are included in this PR?

This PR re-enables the test, so i don't forget about it

Are these changes tested?

I will verify it manually via

cargo bench --profile dev --bench sql_planner -- physical_plan_tpcds_all

Are there any user-facing changes?

No

@alamb alamb marked this pull request as draft September 29, 2025 17:46
@github-actions github-actions bot added the core Core DataFusion crate label Sep 29, 2025
alamb added a commit that referenced this pull request Nov 10, 2025
…ion (#18567)

Note this targets the branch-51 release branch

## Which issue does this PR close?

- part of #17558
- resolves #17801 in the 51
release branch

## Rationale for this change

- We merged some clever rewrites for `coalesce` and `nvl2` to use `CASE`
which are faster and more correct (👏 @chenkovsky @kosiew )
- However, these rewrites cause subtle schema mismatches in some cases
planning (b/c the CASE simplification nullability logic can't determine
the correct nullability in some cases - see
#17801)
- @pepijnve has some heroic efforts to fix the schema mismatch in
#17813 (comment),
but it is non trivial and I am worried about merging it so close to the
51 release and introducing new edge cases

## What changes are included in this PR?

1. Revert #17357 /
e5dcc8c
3. Revert #17991 /
ea83c26
2. Revert #18191 /
22c4214
2. Cherry-pick 6202254, a test that
reproduces the schema mismatch issue (from
#18536)
3. Cherry-pick 735cacf, a fix for the
benchmarks that regressed due to the revert (from
#17833)
4. Update datafusion-testing (see separate PR here) for extended tests
(see apache/datafusion-testing#15)

## Are these changes tested?

Yes I added a new test

## Are there any user-facing changes?

<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->

<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant