andygrove opened a new issue, #6850:
URL: https://github.com/apache/datafusion-comet/issues/6850
### What is the problem the feature request solves?
#6716, which fixes #6613, makes native array and map functions skip their
later arguments on rows where the array or map is NULL, as Spark's
null-intolerant `BinaryExpression`s and `TernaryExpression`s do. It sets
`null_short_circuit` on `ScalarFunc` and `ListExtract`. A few related shapes
are left over:
- **`element_at`** still uses `CometElementAt`'s `CASE WHEN <operand> IS NOT
NULL` guard (#5766). The guard evaluates the operand twice, and under ANSI it
declines a nullable nondeterministic operand. Setting the flag on its
`ListExtract` would evaluate the operand once and let that decline go. This
overlaps #5867.
- **`map_from_arrays`** nests `CASE WHEN keys IS NOT NULL` and `CASE WHEN
values IS NOT NULL`, so it serializes and evaluates both arrays twice. With the
flag, each would be evaluated once. #5854 rewrites this serde, so this should
come after it.
- **`array_contains` on flat float arrays** (#6599) keeps a nullable array
whose value is neither a column nor a literal on the codegen dispatcher,
through `eagerEvalReason` in `CometArrayContains`. Now that
`spark_array_contains` gets the short-circuit, that decline can go.
- **Shared arguments where Spark doesn't eliminate subexpressions.**
`canShortCircuitNulls` doesn't skip an argument that holds a subexpression the
operator's expressions share, because Spark's generated code evaluates those
for every row and raises if one fails. With
`spark.sql.subexpressionElimination.enabled=false`, Spark skips the argument
instead, so Comet raises where Spark returns NULL.
- **`array_repeat`** is the other way around: Spark's `ArrayRepeat.eval`
reads the count before the element, so a left-to-right guard doesn't fit it.
With whole-stage codegen, which is the default, Spark's generated code
evaluates the element first and raises the same way Comet does. On 3.4.3,
`SELECT array_repeat(CAST(s AS INT), cnt)` over a row `('bad', NULL)` raises
`CAST_INVALID_INPUT` in both, so nothing differs in the default configuration.
### Describe the potential solution
Move `element_at`, and `map_from_arrays` after #5854, to
`withNullShortCircuit`. Drop `CometArrayContains.eagerEvalReason`. Decide
whether the shared-argument case should follow
`spark.sql.subexpressionElimination.enabled`, accepting that Spark's aggregate
update path eliminates subexpressions whatever that flag says.
### Additional context
Raised in the #6716 review.
--
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]
