GH-14736: [C++] Propagate struct validity into union children - #51017
Open
Hei-MaoM wants to merge 1 commit into
Open
GH-14736: [C++] Propagate struct validity into union children#51017Hei-MaoM wants to merge 1 commit into
Hei-MaoM wants to merge 1 commit into
Conversation
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.
Rationale for this change
StructArray::GetFlattenedFieldcurrently combines a struct's validitybitmap with a child by assigning it to buffer 0. Union arrays do not have
top-level validity buffers, so flattening a union field aborts even when
the parent bitmap is all-valid. When the parent contains nulls, that
validity must instead be represented by the union's child arrays.
What changes are included in this PR?
are null without invalidating valid slots that share the same original
child offset.
Are these changes tested?
cmake --build /build/cpp --target arrow-array-test -j 4/build/cpp/debug/arrow-array-test --gtest_filter=StructArray.FlattenSparseUnion:StructArray.FlattenDenseUnionWithSharedOffsets --gtest_brief=1/build/cpp/debug/arrow-array-test --gtest_brief=1clang-format 18.1.8cpplint 1.6.1git diff --checkAre there any user-facing changes?
Yes.
StructArray::GetFlattenedFieldand callers such asstruct_fieldand
Flattenno longer crash when selecting a union field from a nullablestruct. Parent nulls are represented in the appropriate union children.
There are no public API changes.
This PR contains a "Critical Fix". It fixes a crash on valid input.
Closes #14736.
AI assistance disclosure
I used OpenAI Codex to help investigate the issue, draft parts of the
implementation and tests, and run verification. I reviewed the final diff
and am responsible for understanding, debugging, and maintaining the change.