andygrove opened a new issue, #6086:
URL: https://github.com/apache/datafusion-comet/issues/6086
## Describe the bug
`CometArrayAppend.convert` reproduces Spark's NULL propagation with
```
CASE WHEN <array> IS NOT NULL THEN array_append(<array>, <item>) ELSE null
END
```
The item sits inside the `THEN` branch. DataFusion's `CaseExpr` evaluates a
`THEN` branch through
`filter_record_batch`, so the item is only evaluated on the rows the guard
selects. Spark's
`ArrayAppend.doGenCode` does the opposite:
```scala
val nullSafeEval =
leftGen.code + rightGen.code + ctx.nullSafeExec(left.nullable,
leftGen.isNull) {
...
}
```
`rightGen.code` is emitted *outside* `nullSafeExec`, so Spark evaluates the
item on every row and
only the result assignment is guarded. (`ArrayAppend.eval` does
short-circuit, so the interpreted
path and the codegen path already disagree with each other in Spark. Under
whole-stage codegen the
codegen path is what runs.)
The consequence is that under ANSI mode an item that raises on a row whose
array is `NULL` raises in
Spark and does not raise in Comet. A query that Spark fails, Comet answers.
This is separate from the nondeterministic-item half of the same problem,
which #5781 covers and
which is being fixed in #5867 by declining a nondeterministic child. A
deterministic item that
raises is not nondeterministic, so that decline does not cover it.
## Steps to reproduce
Reproduced against `main` at `5ca149928`, Spark 3.4.3, as a
`CometSqlFileTestSuite` fixture:
```sql
-- Config: spark.sql.ansi.enabled=true
statement
CREATE TABLE t(_1 int) USING parquet
statement
INSERT INTO t VALUES (0), (1), (2), (3)
query expect_error(DIVIDE_BY_ZERO)
SELECT _1, array_append(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY<INT>)),
1 / (_1 - 1)) AS a
FROM t
```
At `_1 = 1` the array is `NULL` and the item divides by zero. Result:
```
cometError.isDefined was false Expected Comet to throw an error matching
'DIVIDE_BY_ZERO' but query succeeded
```
Spark raises. Comet returns `[NULL]` for that row and completes.
A control query in the same fixture confirms Comet's ANSI divide does raise
on its own, so the test
is not vacuous:
```sql
query expect_error(DIVIDE_BY_ZERO)
SELECT _1, 1 / (_1 - 1) AS d FROM t
```
That one passes.
## Expected behavior
Comet raises `DIVIDE_BY_ZERO`, as Spark does.
## Additional context
`CometArrayAppend.getSupportLevel` currently reports `Compatible()` for this
case, so nothing in the
serde or in the generated compatibility guide records the divergence.
Scope: `ArrayAppend` is `RuntimeReplaceable` on Spark 4.x (rewritten to
`array_insert(-1)`), so
`CometArrayAppend` is only reachable on Spark 3.4 and 3.5.
Two possible fixes:
1. Evaluate the item outside the guard, so the `CASE` only guards the result
rather than the
operand evaluation. This keeps the native path for every item.
2. Widen the decline in `CometArrayAppend.getSupportLevel` to any
non-foldable item under ANSI, and
let `CodegenDispatchFallback` route it to Spark's own `doGenCode`.
Cheaper, but it gives up the
native path for a common shape.
I checked the sibling serdes that use the same guard idiom.
`CometMapFromArrays` builds
`IsNotNull(left) AND IsNotNull(right)` as its guard, which looked like the
same bug in the opposite
direction (Comet raising where Spark's `nullSafeCodeGen` short-circuits). It
does not reproduce:
```sql
query expect_native(map_from_arrays)
SELECT _1, map_from_arrays(IF(_1 % 2 = 0, array(_1), CAST(NULL AS
ARRAY<INT>)), array(1 / (_1 - 1))) AS m
FROM t
```
runs natively and matches Spark. `CometSize` has a single child that the
guard evaluates anyway, and
`CometArraysZip`'s children are all in the guard condition, so
`array_append` looks like the only
one of the four affected.
--
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]
