pepijnve commented on PR #17813:
URL: https://github.com/apache/datafusion/pull/17813#issuecomment-3513800518

   > @pepijnve have you already tried (just) special casing nullability 
reporting in case expressions like ... to be the nullability of `z`?
   > (this is basically a special case for the pattern created by the 
`coalesce` simplification
   > Please forgive me if you have already tried this, I can't remember the 
entire history
   > I realize this PR is the same idea but implements a more general case of it
   
   No worries, it's been a twisty path so far. That was v0.1 I think.
   
   To implement that you need to add some logic in `when` branch loop that 
filters out certain branches where the nullability of the `then` expression is 
not relevant. The simplest check of course is for `w, t` that `w == 
is_not_null(t)`. So that's exactly what I added in a function named 
`const_result_when_value_is_null` which was then used in the loop.
   
   After adding that it was immediately obvious that a number of other 
situations are just as trivial to test, so I thought I might as well add those. 
That's the version in the first commit 
`408cee1ccd544fa6e7d17b52582129bb86c362e6`.
   
   It was 
https://github.com/apache/datafusion/pull/17813#discussion_r2388211726 that got 
me thinking that this can be made even more general. The other feedback I got 
in between then and now is what led to the current state of the PR.
   
   I've been familiarising myself with more of the code in `optimizer` as part 
of this work. I think one thing we could consider is to move 
`GuaranteeRewriter` to `expr-common` and then use that to rewrite the `when` 
expression. I avoided rewriting the logical expr out of a concern that this 
might be too wasteful, but the code would definitely be simpler (drop the 
callback function stuff) if we rewrite and then determine the predicate bounds.
   
   Even better would be to also leverage the simplification and const 
evaluation from `optimizer`, but since that requires a dependency on 
`physical-expr` that's going to be hard to do.


-- 
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