dwsmith1983 opened a new pull request, #7589: URL: https://github.com/apache/datafusion-comet/pull/7589
## Which issue does this PR close? Part of #6970. This is the second of three pull requests for it. These build on #5867, which adds the shared-subexpression check they extend, so #5867 merges first. Order after it: 1. #7586: which operators share a subexpression the way Spark does. 2. This one: only a shared subexpression that can fail stops a NULL guard from skipping it. 3. The last pull request: the element_at and map_from_arrays fix, whose merge finishes #6970. They target `main`, so until #5867 merges each one also shows its commits, and the ones before it. ## Rationale for this change Skipping a shared subexpression on a NULL row only changes the result when evaluating it would raise. #7586 and #5867 treat every shared subexpression as one that matters, so a lookup keyed on a shared `CAST(s AS INT)` without ANSI, or on `lower(c)`, falls back or is evaluated for every row for no reason. ## What changes are included in this PR? - A conservative allowlist decides whether an expression can fail, following the eval mode: comparisons, NULL checks, boolean logic, constructors, string case conversion, trim, substring, length, hashes and date parts, plus arithmetic and casts outside ANSI, widening casts other than to a timestamp, and casts of a literal that evaluate. Anything else counts as able to fail. - Only a shared subexpression that can fail stops a NULL guard from skipping an argument, and only the arguments after the first nullable one can be skipped. - The codegen dispatcher's check uses the same rule. ## How are these changes tested? `map_null_short_circuit_shared.sql` and additions to the non-ANSI and uncertain files check that these shapes stay native, and that a shared key that fails in any mode still raises as Spark does. Each fails on #7586. Unit tests cover the allowlist in both eval modes, including a date to timestamp cast, which can overflow, and Comet's decimal overflow check. Two existing non-ANSI queries now dispatch their lookups instead of falling back. The suites and TPC-DS plan stability pass on Spark 3.5 and 4.1, and the new file passes on 4.2. -- 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]
