dwsmith1983 opened a new pull request, #5867:
URL: https://github.com/apache/datafusion-comet/pull/5867
## Which issue does this PR close?
Closes #5580, closes #5781.
## Rationale for this change
Two serde gaps in the array and map expressions, both resolved by the
codegen dispatcher rather than by native changes.
`map_col[key]` and `element_at(map, key)` decline float, collated and
complex map keys because the native lookup compares raw Arrow values where
Spark normalizes `-0.0`, treats NaN as equal to itself, compares strings by
collation and compares complex keys with interpreted ordering. The declines are
right, but neither serde mixed in `CodegenDispatchFallback`, so the whole
projection fell back to Spark instead of running Spark's own generated code
inside the Comet pipeline.
`size`, `array_append`, `arrays_zip` and `map_from_arrays` reproduce Spark's
NULL propagation with a `CASE WHEN child IS NOT NULL` guard that serializes the
child twice. A stateful child advances each copy independently, so the guard
and the operation see different rows and the answer is silently wrong: on a
16-row table with `IF(monotonically_increasing_id() % 2 = 0, array(1), NULL)`
as the operand, `size` returned -1 on five rows where Spark returns 1,
`arrays_zip` returned `[null, 2]` for `[1, 2]`, `array_append` returned `[2]`
for `[1, 2]` and `map_from_arrays` returned NULL for `{1 -> 2}`. `element_at`
had the same shape and was fixed in #5766 for its ANSI arm; these four are not
ANSI-gated, so the wrong answers were reachable in every configuration.
## What changes are included in this PR?
- `CometMapExtract` and `CometElementAt` mix in `CodegenDispatchFallback`,
so the declined key types run through the dispatcher. `CometElementAt`'s ANSI
arm for a nondeterministic operand dispatches the same way.
- A shared `NullGuardSupport` gate declines any nondeterministic child in
`CometSize`, `CometArrayAppend` (the array operand only; the item is not under
the guard), `CometArraysZip` and `CometMapFromArrays`, and all four mix in
`CodegenDispatchFallback`, so Spark's generated code evaluates the child once.
Nullability is not consulted: a non-nullable stateful child only stays correct
today because DataFusion skips the filter when the guard matches every row,
which is not a contract to rely on.
- `getUnsupportedReasons` lists updated for the doc generator, and the six
affected rows in the expressions guide move from Native to Hybrid.
## How are these changes tested?
SQL-file fixtures over parquet tables, all asserting Spark's answer and the
execution path:
- Four new `*_nondeterministic_child.sql` fixtures, one per serde, with the
stateful operand (dispatched), a non-nullable stateful operand (dispatched),
and a deterministic nullable operand (native). `array_append` also pins that a
stateful item stays native, and its fixture is capped at Spark 3.5 because 4.x
rewrites `array_append` to `array_insert`. Before the change, the stateful
cases failed as result mismatches with the values above.
- `element_at_map.sql` and `get_map_value.sql` flip their fallback cases to
dispatch and add NaN lookups and a struct-keyed map column with a per-row key,
a NULL inside the key and a NULL key. `element_at_map_collation.sql` flips the
same way and passes on the Spark 4.0 profile. `element_at_ansi.sql` pins the
dispatched nondeterministic arm.
- `map_from_arrays_dedup_policy.sql` disables the dispatcher so its LAST_WIN
fallback assertion keeps meaning what it says, matching the existing
`map_from_entries` fixture.
- `CometMapExpressionSuite` renames five fallback tests to dispatch tests
through the helper that checks the dispatch tag.
578 of 578 across `CometSqlFileTestSuite`, `CometArrayExpressionSuite` and
`CometMapExpressionSuite` on Spark 3.5, the map fixtures on the Spark 4.0
profile, and `test-compile` on 4.0.
--
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]
