alamb opened a new pull request, #18567:
URL: https://github.com/apache/datafusion/pull/18567

   Note this targets the branch-51 release branch
   
   ## Which issue does this PR close?
   
   - part of https://github.com/apache/datafusion/issues/17558
   - resolves https://github.com/apache/datafusion/issues/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 
https://github.com/apache/datafusion/issues/17801)
   - @pepijnve has some heroic efforts to fix the schema mismatch in 
https://github.com/apache/datafusion/pull/17813#issuecomment-3508025785, 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 https://github.com/apache/datafusion/pull/17357 / 
e5dcc8c04f9559f8af6efea3c7ff8202f3c1c618
   3. Revert https://github.com/apache/datafusion/pull/17991 / 
ea83c2644eb559e55401ce2f7f975032e8d7845d
   2. Revert https://github.com/apache/datafusion/pull/18191 / 
22c4214fe1ca3953932f3f12ccd5b68dbfbefdf3
   2. Cherry-pick 62022545af09346ed578e2c7c61a7c897e2ccfc0, a test that 
reproduces the schema mismatch issue (from 
https://github.com/apache/datafusion/pull/18536)
   3. Cherry-pick 735cacf71d8632d786bfb920126ce6719c42156d, a fix for the 
benchmarks that regressed due to the revert (from 
https://github.com/apache/datafusion/pull/17833)
   4. Update datafusion-testing (see separate PR here) for extended tests
   
   ## Are these changes tested?
   
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are 
they covered by existing tests)?
   -->
   
   ## 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.
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to