GH-50879: [C++] Implement replace_with_mask for List and LargeList types - #50880
GH-50879: [C++] Implement replace_with_mask for List and LargeList types#50880pratyushadk wants to merge 2 commits into
Conversation
…ist types This commit adds support for variable-width list types (ListType and LargeListType) to the replace_with_mask compute kernel. It introduces a specialization of ReplaceMaskImpl that handles variable-length children safely by directly iterating over values and appending array slices, avoiding invalid length mutations.
|
|
There was a problem hiding this comment.
Pull request overview
This PR extends the replace_with_mask compute kernel to support variable-width list arrays (ListType and LargeListType) in Arrow C++. It adds a dedicated ReplaceMaskImpl specialization for list types that builds output via list builders (avoiding unsafe direct offset mutation) and registers the new list kernels specifically for replace_with_mask while keeping fill_null_forward/backward registrations list-free.
Changes:
- Added a
ReplaceMaskImplspecialization forListType/LargeListTypeusingListBuilder/LargeListBuilderandAppendArraySliceto copy list elements safely. - Adjusted the primary
ReplaceMaskImplSFINAE condition to exclude variable-length list types, preventing template ambiguity on MSVC. - Registered
LISTandLARGE_LISTkernels forreplace_with_maskonly, and added new unit tests covering list/large_list scenarios (including chunked inputs).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cpp/src/arrow/compute/kernels/vector_replace.cc | Adds list/large_list kernel implementation + registration adjustments to enable replace_with_mask on variable-width list types. |
| cpp/src/arrow/compute/kernels/vector_replace_test.cc | Adds C++ unit tests validating replace_with_mask behavior for list<int32> and large_list<int32> (scalar/array masks, nulls, chunked, mismatch cases). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The |
Rationale for this change
Currently, the
replace_with_maskcompute kernel throws aNotImplementederror when called on variable-width list arrays (ListTypeandLargeListType). Given that this function is widely used for data manipulation, supporting list arrays natively is a highly requested enhancement.What changes are included in this PR?
This PR introduces variable-width list support to the
replace_with_maskkernel.ReplaceMaskImpltemplate condition to exclude variable-length list types to resolve MSVC compilation ambiguities.ReplaceMaskImplforListTypeandLargeListType. It safely iterates through values and usesListBuilder/LargeListBuilderto append array slices of the child arrays, handling nulls without directly mutating array offsets.ListandLargeListtypes to thereplace_with_maskfunction registry (excludingfill_nullfunctions which do not yet support lists).Are these changes tested?
Yes. Extensive C++ unit tests were added in
vector_replace_test.ccfor bothListTypeandLargeListType. The test suites cover scalar vs. array masks, null mask entries, empty lists, mismatched replacement lengths, and chunked arrays. All tests pass locally on Windows MSVC.Are there any user-facing changes?
Yes. Users can now pass
ListArrayandLargeListArraytypes toreplace_with_maskwithout encountering aNotImplementederror. There are no breaking changes to existing public APIs.