fix: dispatch map lookups with normalized keys and nondeterministic null-guarded children - #5867
Open
dwsmith1983 wants to merge 1 commit into
Open
Conversation
…ull-guarded children Map lookups with float, collated or complex keys were declined by the serde without a dispatch fallback, so the whole projection fell back to Spark. Four array and map serdes reproduce NULL propagation with a guard that serializes the child twice, so a stateful child was evaluated twice natively and returned wrong answers. Mix the codegen dispatch fallback into the map lookups, and decline any nondeterministic child in size, array_append, arrays_zip and map_from_arrays through one shared gate so Spark's generated code evaluates it once. Closes apache#5580 Closes apache#5781
dwsmith1983
force-pushed
the
fix/serde-map-keys-and-nondeterministic-children
branch
from
September 11, 2026 17:11
a4ff6c6 to
ff812cc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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]andelement_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 inCodegenDispatchFallback, so the whole projection fell back to Spark instead of running Spark's own generated code inside the Comet pipeline.size,array_append,arrays_zipandmap_from_arraysreproduce Spark's NULL propagation with aCASE WHEN child IS NOT NULLguard 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 withIF(monotonically_increasing_id() % 2 = 0, array(1), NULL)as the operand,sizereturned -1 on five rows where Spark returns 1,arrays_zipreturned[null, 2]for[1, 2],array_appendreturned[2]for[1, 2]andmap_from_arraysreturned NULL for{1 -> 2}.element_athad 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?
CometMapExtractandCometElementAtmix inCodegenDispatchFallback, so the declined key types run through the dispatcher.CometElementAt's ANSI arm for a nondeterministic operand dispatches the same way.NullGuardSupportgate declines any nondeterministic child inCometSize,CometArrayAppend(the array operand only; the item is not under the guard),CometArraysZipandCometMapFromArrays, and all four mix inCodegenDispatchFallback, 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.getUnsupportedReasonslists 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:
*_nondeterministic_child.sqlfixtures, one per serde, with the stateful operand (dispatched), a non-nullable stateful operand (dispatched), and a deterministic nullable operand (native).array_appendalso pins that a stateful item stays native, and its fixture is capped at Spark 3.5 because 4.x rewritesarray_appendtoarray_insert. Before the change, the stateful cases failed as result mismatches with the values above.element_at_map.sqlandget_map_value.sqlflip 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.sqlflips the same way and passes on the Spark 4.0 profile.element_at_ansi.sqlpins the dispatched nondeterministic arm.map_from_arrays_dedup_policy.sqldisables the dispatcher so its LAST_WIN fallback assertion keeps meaning what it says, matching the existingmap_from_entriesfixture.CometMapExpressionSuiterenames five fallback tests to dispatch tests through the helper that checks the dispatch tag.578 of 578 across
CometSqlFileTestSuite,CometArrayExpressionSuiteandCometMapExpressionSuiteon Spark 3.5, the map fixtures on the Spark 4.0 profile, andtest-compileon 4.0.