dwsmith1983 opened a new pull request, #7586: URL: https://github.com/apache/datafusion-comet/pull/7586
## Which issue does this PR close? Part of #6970. This is the first 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. This one: which operators share a subexpression the way Spark does. 2. The next pull request: 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 #5867 tells the serdes which subexpressions an operator shares, so a native NULL guard does not skip one that Spark evaluates for every row. It treats every operator like a projection. Spark shares that way only in a projection, an aggregate's function inputs, shuffle keys, the TopK projection and the prefix of a sort's first key. In a filter, a join, an aggregate's results and the other operators, whether Spark shares depends on the version (4.2 added it to `FilterExec`), on whole-stage codegen and on the join side. Evaluating a shared argument for every row there can raise `DIVIDE_BY_ZERO` on a row where Spark returns a result. ## What changes are included in this PR? - `withSharedSubexpressions` takes a required `certain` flag. Projections, aggregate function inputs, shuffle keys, the TopK projection and a sort's first key are certain. Other operators and aggregate results are not. - Where sharing is not certain, the `withNullShortCircuit` callers and `GetArrayItem` fall back to Spark when an argument they would skip holds a shared subexpression, instead of evaluating it for every row. Each lists the reason. - A sort's scope is its first key, since its later keys are compared one by one. An aggregate's function inputs and results get scopes of their own. ## How are these changes tested? `map_null_short_circuit_uncertain.sql` covers a filter, a join condition and join keys, sorts, and an aggregate's inputs and results. Each new query fails on #5867's code or when the change behind it is undone. The join checks turn adaptive execution off because of #6442. A unit test covers the flag. The ORDER BY queries in `map_lookup_shared_subexpression_operators.sql` now dispatch their lookups instead of falling back. The suites and TPC-DS plan stability pass on Spark 3.5 and 4.1, with no plan changes, 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]
