fix: preserve nested collection expression semantics - #5545
Closed
sunchao wants to merge 20 commits into
Closed
Conversation
Evaluate the collection once and evaluate the key only for non-null rows. This covers element_at, bracket lookup, and try_element_at in both ANSI modes without duplicating stateful operands through a CASE expression. Keep the existing map lookup and type coercion, preserve scalar broadcasting, and mask null-map rows. Add native and Spark regressions for throwing keys, stateful operands, null inputs, and batch shape.
Dispatch collated array_contains operands through Spark codegen, including nested arrays and structs. Check collations before floating-point opt-in so allowIncompatible cannot select a kernel that loses string collation. Cover folded integer-key maps with collated values, case-sensitive controls, nulls, nested values, and both compatibility settings. Update collated-key membership coverage to expect the safe codegen path.
Keep deeply nullable children while constructing an array, then restore its declared nested element type. This prevents metadata widening from breaking downstream slice and conditional expressions. Cover folded and dynamic map arrays and conditional branches on all-true, all-false, and mixed batches. Retain ArrayInsert's separate result-type normalization and refresh its comments.
Include a NULL array element so Spark 3.4, 3.5, and 4.0 retain the nullable result of element_at for a NULL array in ANSI mode. The stateful index still distinguishes lazy evaluation: eager evaluation selects the NULL second element rather than 7.
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
2 participants
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 #5544.
This is a draft follow-up to #5452 and leaves that PR unchanged, following the agreement to address the remaining cases separately. It depends on #5452: the current main-based diff temporarily includes that PR's unmerged changes. The intended follow-up is these three fix commits. They need to be rebased onto main after #5452 merges and validated again before this draft is marked ready.
Rationale for this change
The remaining cases in #5544 can return incorrect stateful lookup or collation results, evaluate a throwing key for a null collection, or fail because a constructed array's nested type differs from its declared result type. They require coverage beyond the existing SQL fixtures, which disable constant folding and normally run without ANSI mode.
What changes are included in this PR?
try_element_atas well as ordinaryelement_at.array_containscomparisons through Spark codegen, including nested arrays/structs and mixed collated-string/floating-point values.allowIncompatible=truedoes not bypass the collation safeguard.sliceor conditional expressions consume it. RetainArrayInsert's separate result-type normalization.The fixes are separate commits (
f6057825c,6ce68f786, and732e7acfc). The branch also merges main at98cd8c967, preserving its unconditional aggregate-nullability normalization when resolving the planner conflict.How are these changes tested?
Added native and Spark regression coverage for null/empty/scalar collections, selected stateful evaluation across batches, throwing keys, folded map literals, nested collations, and all-true/all-false/mixed conditional batches. Stateful Spark fixtures use one small ordered Parquet input so both rows exercise the same batch.
Local validation on the combined branch, using Spark 4.1.3, JDK 17, and the rebuilt native development library:
Native code was built before the root-reactor JVM tests; the tests used current compiled sources without serializer overlays. This is 270 focused local checks, not a full-project, release-profile, cross-Spark-version, or live Celeborn cluster run. Hosted CI is checked separately.