Skip to content

GH-51612: [C++] Fix list_view to list cast - #51647

Open
rajaryan2007 wants to merge 6 commits into
apache:mainfrom
rajaryan2007:fix-gh-51612-list-view-cast
Open

rajaryan2007 wants to merge 6 commits into
apache:mainfrom
rajaryan2007:fix-gh-51612-list-view-cast

Conversation

@rajaryan2007

@rajaryan2007 rajaryan2007 commented Sep 29, 2026 •

Copy link
Copy Markdown

Rationale for this change

Casting a list_view to a list (or across offset widths) incorrectly reused the offsets buffer and ignored the sizes buffer. This caused out-of-bounds memory reads (crashes) and generated invalid data whenever views were overlapping or out-of-order.

views = pa.ListViewArray.from_arrays(
    pa.array([2, 0, 1], pa.int32()),   # offsets
    pa.array([2, 2, 1], pa.int32()),   # sizes
    pa.array([1, 2, 3, 4], pa.int32()))
views.cast(pa.list_(pa.int32())).to_pylist()

# Before: [[], [1], []]         (Invalid: negative / non-monotonic offsets)
# After:  [[3, 4], [1, 2], [2]] (Correct)

### What changes are included in this PR?
list_util::internal::ListFromListView: New shared internal helper that materializes a standard list layout by gathering the values referenced by each view. Supports all 4 offset-width combinations.

CastList Update: List-view sources now use this helper before casting the child array.

FromListView Fix: ListArray::FromListView and LargeListArray::FromListView now delegate to this shared helper. This removes duplicated code (-31 lines) and fixes a bug where nulls were handled incorrectly for sliced inputs (GH-51613).

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:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Close #51613

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #51612 has been automatically assigned in GitHub to PR creator.

@jonasdedden

jonasdedden commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

@pitrou @felipecrv @kou could one of you take a look at this when you have a moment? It touches the list-view code (ListArray::FromListView and the list-view -> list cast) that you wrote or reviewed originally.

Two points that probably need a committer's call:

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).
@rajaryan2007
rajaryan2007 force-pushed the fix-gh-51612-list-view-cast branch from c2697d7 to b450c3b Compare October 12, 2026 19:36
Comment thread cpp/src/arrow/util/list_util.cc Outdated
Comment thread cpp/src/arrow/util/list_util.cc Outdated
@dmitry-chirkov-dremio

Copy link
Copy Markdown
Contributor

In Debug build, casting list_view<run_end_encoded<int32, extension>> aborts with Check failed: array.type->Equals(type_) (builder_run_end.cc:255). The builder has the storage type, but values has the extension type. Please fix it and add a test.
In the description, close the code block and add * Closes: #51613.

…ypes

ListFromListView builds the gathered values with a builder created from the
storage type of the view's value type, because MakeBuilder does not support
extension types. The appended slices, however, kept the original type, and a
builder may check or inspect the type of the array it is given: the run-end
encoded builder aborts (builder_run_end.cc) and the dictionary builder casts
the type unchecked. Rewrite the type of the appended span tree to those same
storage types, which the result's types are restored from afterwards.
A run-end encoded storage is the only storage type whose builder checks the
type of the array it appends, so cover gathering through it: a plain run-end
encoded child, an extension of one (the abort reported in apacheGH-51612), an
extension nested in the value type of the runs, and one behind a struct, which
the struct builder forwards to the run-end encoded field builder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++] ListArray::FromListView gives wrong nulls for sliced list-view arrays

3 participants