GH-51612: [C++] Fix list_view to list cast - #51647
Open
rajaryan2007 wants to merge 1 commit into
Open
rajaryan2007 wants to merge 1 commit into
rajaryan2007 wants to merge 1 commit into
Conversation
CastList reused the view's offsets buffer as the list offsets and ignored the sizes, so out-of-order or overlapping views came out wrong. Gather the referenced values into a real list layout instead, reusing the shared helper in FromListView (which also fixes apacheGH-51613).
|
|
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
Casting a
list_viewto alist(or across offset widths) incorrectly reused the offsets buffer and ignored thesizesbuffer. This caused out-of-bounds memory reads (crashes) and generated invalid data whenever views were overlapping or out-of-order.Are these changes tested?
Yes. Added and updated tests in C++ (scalar_cast_test.cc, list_util_test.cc, array_list_test.cc) and Python (test_array.py). Tests cover all offset-width combinations, null views, out-of-order/overlapping views, sliced inputs, zero-length edge cases, and nested list-views.
Are there any user-facing changes?
Yes.
Performance: list_view -> list casts are no longer zero-copy. The referenced values are now always gathered into a new child array. This is a necessary performance cost to guarantee correctness.
This PR includes breaking changes to public APIs.
ListArray::FromListView and LargeListViewArray::FromListView now return correct nulls for sliced inputs. This changes the output for users who may have been relying on the previous bugged behavior.
This PR contains a "Critical Fix".
Yes, this fixes a bug that produced incorrect and invalid data (negative/non-monotonic offsets that fail validation) and prevents Check failed: (off) <= (length) abort crashes caused by out-of-bounds reads.
Was AI used for this PR?
I used AI to help explain concepts, generate some inline code comments, and assist with writing and building tests.
PR code and description written by:
Reviewed before submission by: