peterxcli commented on PR #5854:
URL:
https://github.com/apache/datafusion-comet/pull/5854#issuecomment-5723418072
@andygrove I'd land this PR first, then #5867 rebased onto it, and close
#5844 and #5846 as superseded.
**Ordering.** datafusion-spark 55.1's `map_deduplicate_keys` fixes a key's
slot at its first occurrence and overwrites the value in place under LAST_WIN
(`function/map/utils.rs`); `str_to_map` does the same. `['a', 'b', 'a']` with
`[1, 2, 3]` gives `{a -> 3, b -> 2}`, as Spark does. 855b039 pins this with
Rust unit tests for all three builders and fixture queries through `map_keys` /
`map_values`.
**#5844.** This branch resolves #5589 directly: `map_from_arrays` runs
natively under LAST_WIN, so the dispatch route, and the 6 to 10% slowdown your
review measured on it, is no longer needed. Its serde, fixture, test and
benchmark all assume that route. Its extra fixture cases are covered here now
(NULL as the winning value, NULL and empty arrays, a length mismatch in either
direction). @LinSimon-901101, sorry for the collision; I'd suggest closing it
once this lands.
**#5846.** Both of its fixes are carried here. The datafusion-spark kernel
validates lengths per row, and the wrapper raises
`SparkError::MapKeyValueDiffSizes`, as you asked for on #5846. For null
short-circuiting, @sunchao's test showed the old `AND` guard is not enough,
since DataFusion's `AND` evaluates its right side on the whole batch unless the
left side is false on all or most rows; 6d1d327 uses the nested `CaseWhen`
shape from #5846 instead. All three of its Scala tests pass verbatim on this
branch. I can port them with attribution if that is preferred over rebasing
#5846 down to them.
**#5867.** Still needed: the nested guards serialize each child twice, so
the `NullGuardSupport` gate applies unchanged. The conflict is only its
`CometMapFromArrays` shape, which wraps the LAST_WIN `Incompatible` branch this
PR deletes, and the two dedup fixtures that assert dispatch. Rebased onto this
PR it becomes
`NullGuardSupport.nondeterministicChild(expr.children).getOrElse(MapBuilderSupport.keySupport(expr.dataType.keyType))`,
with those dispatch assertions dropped and
`map_from_arrays_nondeterministic_child.sql` kept. The
`CodegenDispatchFallback` mixin is still worth keeping there for the declines
that remain, collated keys and nondeterministic children.
--
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]
