dwsmith1983 opened a new pull request, #7590: URL: https://github.com/apache/datafusion-comet/pull/7590
## Which issue does this PR close? Closes #6970. This is the last 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. #7589: only a shared subexpression that can fail stops a NULL guard from skipping it. 3. This one: 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 With ANSI on, Spark evaluates a subexpression its operator shares for every row, before any NULL check, so it raises on a row whose array or map is NULL. Native `element_at` and `map_from_arrays` skipped the argument on those rows and returned rows where Spark fails. ## What changes are included in this PR? Where an argument a NULL guard would skip holds a shared subexpression that can fail, `element_at`, `GetArrayItem`, `GetMapValue` and the other `withNullShortCircuit` callers now: - evaluate it for every row when everything else in the argument cannot fail and the operator's sharing is certain, which is what Spark does; - otherwise fall back to Spark, for example when the argument also holds an unshared part that can fail, which Spark skips on that row. `map_from_arrays` has no native form that evaluates its values for every row, since native `map` takes NULL rows from the keys alone, so it falls back in that case. Outside an operator's scope, `element_at` and `map_from_arrays` keep their guards. ## How are these changes tested? Three new files cover `element_at` on arrays and maps and `map_from_arrays`, in a projection, an aggregate and a filter, with elimination on and off, including the issue's repro. The error queries fail on #7589, where Comet returned rows. Unit tests cover the eager check and the guards outside a scope. The suites and TPC-DS plan stability pass on Spark 3.5 and 4.1, with no plan changes, and the new files pass 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]
