pepijnve commented on PR #17813: URL: https://github.com/apache/datafusion/pull/17813#issuecomment-3348010678
> I am not quite sure about this implementation (I am hoping #17628 might solve the problem too with more sophisticated case folding) I warned you it wasn't very elegant. :smiling: I don't think #17628 covers the same thing though. What we're trying to do here is get `ExprSchemable::nullable` to report an accurate value outside of the optimiser. Ideally you would want that to work both before and after case folding. I agree that there is a lot of overlap in the required logic though. If you were to take the case expression, replace the `then` expression trees with literal `NULL` everywhere they occur, then perform case folding, and then perform null analysis then you would get the same result. > My only real concern is that the newly added tests cover only the new code, and not the "end to end" behavior you tracked down (namely that the case pattern with coalesce changes the nullability). > > Would it be possible to add some of the cases as expr simplification tests too? Somewhere like here? I'm not sure what kind of test you have in mind. The end to end case is (admittedly very indirectly) covered by TPC-DS query 75 and the removal of the double optimisation. If you revert the production code change in this PR, but keep the test change you'll see that it fails. For the simplifier itself, I was wondering if there shouldn't be some internal assertions that verifies that the result of calling `ScalarUDF::simplify` doesn't change key aspects of the expression that the schema also encodes. Both the data type and the nullability should not change since that causes the expression and the schema to be mismatched. -- 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]
