dwsmith1983 commented on code in PR #5867:
URL: https://github.com/apache/datafusion-comet/pull/5867#discussion_r4270726034
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -788,7 +865,7 @@ object CometArrayReverse extends
CometExpressionSerde[Reverse] with ArraysBase {
}
-object CometElementAt extends CometExpressionSerde[ElementAt] {
+object CometElementAt extends CometExpressionSerde[ElementAt] with
CodegenDispatchFallback {
Review Comment:
> Could this route preserve eager shared-expression evaluation, or retain
Spark fallback for this shape?
It keeps Spark fallback for it. `QueryPlanSerde.skipsSharedSubexpression`
looks, inside the operator's `withSharedSubexpressions` scope and with
subexpression elimination on, for a node in the expression that can skip an
argument holding a subexpression the operator shares. For most nodes that is
any argument after the first. Division and `pmod` evaluate the divisor first
and skip the dividend, and `locate` skips its first two arguments when `start`
is NULL. The six serdes this PR newly dispatches decline in that case, so
`element_at(m1, 1.0D/x), element_at(m2, 1.0D/x)` falls back to Spark as it did
at the base. A subexpression the expression repeats on its own stays
dispatched, since the kernel eliminates it too and evaluates it for every row.
Declining exposed two plan-time checks that ran outside that scope while
serialization ran inside it: the native shuffle key check and the
`TakeOrderedAndProject` projection check. Without a fix, `DISTRIBUTE BY` or a
sort on such a pair would have chosen native shuffle and then failed at
execution. Both now use the scope the writer and `finalNativePlan` use.
The native `element_at` and `map_from_arrays` guards have the same gap on
main with INT keys, which is #6970, and the other dispatch serdes on main have
it as well, which is #6971.
--
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]