eejbyfeldt commented on code in PR #13032: URL: https://github.com/apache/datafusion/pull/13032#discussion_r1810640566
########## datafusion/sqllogictest/test_files/cse.slt: ########## @@ -199,18 +199,18 @@ physical_plan # Surely only once but also conditionally evaluated subexpressions query TT EXPLAIN SELECT - (a = 1 OR random() = 0) AND (a = 1 OR random() = 1) AS c1, - (a = 2 AND random() = 0) OR (a = 2 AND random() = 1) AS c2, + (a = 1 OR random() = 0) AND (a = 2 OR random() = 1) AS c1, Review Comment: > (Actually the same trick is applied in the previous test called # Surely only once but also conditionally evaluated expressions a bit above.) That was also changed by me in https://github.com/apache/datafusion/pull/12746 for basically the same reason for another rule. > Out of curiosity, volatile common expressions don't get simplified, do they? > E.g. `(a = 1 AND random() = 0) OR (a = 2 AND random() = 0)` is not rewritten to `random() = 0 AND (a = 1 OR a = 2)`? It would end up being rewritten. I agree that does not sound correct. It should probably be addressed separatly as the issue applies to many of the other simplification rules. Created https://github.com/apache/datafusion/issues/13060 -- 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]
