sunchao commented on code in PR #5867:
URL: https://github.com/apache/datafusion-comet/pull/5867#discussion_r4266282723
##########
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:
[P2] [P2] Preserve shared-expression evaluation when dispatching map
lookups. With ANSI and `spark.sql.subexpressionElimination.enabled=true`, a
Parquet table containing nullable DOUBLE-key maps `m1`, `m2` and `x = 0` makes
`SELECT element_at(m1, 1.0D/x), element_at(m2, 1.0D/x) FROM t` raise
`DIVIDE_BY_ZERO` in Spark even when both maps are NULL. Spark’s `ProjectExec`
evaluates the shared division before the lookup NULL guards. This newly enabled
route serializes each lookup separately, so both generated kernels skip the
division and return NULL, allowing the query to succeed instead. At the base,
DOUBLE-key lookups fall back to Spark. Could this route preserve eager
shared-expression evaluation, or retain Spark fallback for this shape?
`canShortCircuitNulls` already identifies the constraint, but dispatch bypasses
it.
Evidence: Reproduced with freshly compiled head
serializers/planner/dispatcher on Spark 4.1.3 in
`/tmp/review5867-d1302-current-_xjobhhq/SharedDeterministicProbe.scala`, with
results in `SharedDeterministicProbe.log`. Input equivalent: `CREATE TABLE t(m1
MAP<DOUBLE,INT>, m2 MAP<DOUBLE,INT>, x DOUBLE) USING parquet; INSERT INTO t
VALUES (NULL, NULL, 0.0D);`. Spark raises `DIVIDE_BY_ZERO` for the two-lookup
query. Either lookup alone returns NULL, and disabling subexpression
elimination makes the pair return `[NULL,NULL]`. With elimination enabled, the
exact-head planner reports `canShortCircuitNulls=false` yet emits both JVM
dispatch expressions. Evaluating those serialized expressions returns NULL for
each column. Spark’s `ProjectExec.doConsume` confirms eager common-expression
evaluation. This is a component reproduction, not a full native execution run.
--
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]
