Skip to content

Commit 542eb21

Browse files
committed
GH-50879: [C++] Implement replace_with_mask for List and LargeList 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.
1 parent e611f48 commit 542eb21

2 files changed

Lines changed: 348 additions & 3 deletions

File tree

cpp/src/arrow/compute/kernels/vector_replace.cc

Lines changed: 124 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
#include "arrow/array/builder_nested.h"
1819
#include "arrow/compute/api_scalar.h"
1920
#include "arrow/compute/kernels/codegen_internal.h"
2021
#include "arrow/compute/kernels/common_internal.h"
@@ -119,7 +120,8 @@ struct ReplaceMaskImpl {};
119120

120121
template <typename Type>
121122
struct ReplaceMaskImpl<
122-
Type, enable_if_t<!(is_base_binary_type<Type>::value || is_null_type<Type>::value)>> {
123+
Type, enable_if_t<!(is_base_binary_type<Type>::value || is_null_type<Type>::value ||
124+
is_var_length_list_type<Type>::value)>> {
123125
static Result<int64_t> ExecScalarMask(KernelContext* ctx, const ArraySpan& array,
124126
const BooleanScalar& mask, ExecValue replacements,
125127
int64_t replacements_offset, ExecResult* out) {
@@ -322,6 +324,111 @@ struct ReplaceMaskImpl<Type, enable_if_base_binary<Type>> {
322324
}
323325
};
324326

327+
// Specialization for variable-size list types (list<T> and large_list<T>).
328+
// Each list element is copied individually using AppendArraySlice, mirroring
329+
// the enable_if_base_binary specialization's per-element builder approach.
330+
template <typename Type>
331+
struct ReplaceMaskImpl<Type, enable_if_var_size_list<Type>> {
332+
using offset_type = typename Type::offset_type;
333+
using BuilderType = typename TypeTraits<Type>::BuilderType;
334+
335+
static Result<int64_t> ExecScalarMask(KernelContext* ctx, const ArraySpan& array,
336+
const BooleanScalar& mask,
337+
ExecValue replacements,
338+
int64_t replacements_offset,
339+
ExecResult* out) {
340+
if (!mask.is_valid) {
341+
// mask = null: output is all-null array
342+
ARROW_ASSIGN_OR_RAISE(
343+
auto replacement_array,
344+
MakeArrayOfNull(array.type->GetSharedPtr(), array.length, ctx->memory_pool()));
345+
out->value = std::move(replacement_array->data());
346+
return replacements_offset;
347+
} else if (mask.value) {
348+
// mask = true: output = replacement
349+
if (replacements.is_scalar()) {
350+
ARROW_ASSIGN_OR_RAISE(
351+
auto replacement_array,
352+
MakeArrayFromScalar(*replacements.scalar, array.length, ctx->memory_pool()));
353+
out->value = std::move(replacement_array->data());
354+
} else {
355+
// Zero-copy slice into the replacements array — same approach as base binary.
356+
std::shared_ptr<ArrayData> result = replacements.array.ToArrayData();
357+
result->offset += replacements_offset;
358+
result->length = array.length;
359+
// Null count from original replacements applies to the whole array, not this
360+
// slice; mark as unknown so it is recomputed on demand.
361+
result->null_count = kUnknownNullCount;
362+
out->value = result;
363+
}
364+
return replacements_offset + array.length;
365+
} else {
366+
// mask = false: output = input (zero-copy)
367+
out->value = array.ToArrayData();
368+
return replacements_offset;
369+
}
370+
}
371+
372+
static Result<int64_t> ExecArrayMask(KernelContext* ctx, const ArraySpan& array,
373+
const ArraySpan& mask, int64_t mask_offset,
374+
ExecValue replacements,
375+
int64_t replacements_offset,
376+
ExecResult* out) {
377+
// Build the output list array element-by-element. We cannot pre-allocate a flat
378+
// buffer (unlike fixed-width types) because the child-array size of each list slot
379+
// is unknown in advance, so we use a list builder and copy slots individually.
380+
std::unique_ptr<ArrayBuilder> raw_builder;
381+
RETURN_NOT_OK(MakeBuilderExactIndex(ctx->memory_pool(),
382+
array.type->GetSharedPtr(), &raw_builder));
383+
auto& builder = checked_cast<BuilderType&>(*raw_builder);
384+
RETURN_NOT_OK(builder.Reserve(array.length));
385+
386+
// Source offset tracks our position in `array` (the values argument).
387+
int64_t source_offset = 0;
388+
389+
// Narrow the mask span to [mask_offset, mask_offset + array.length).
390+
ArraySpan adjusted_mask = mask;
391+
adjusted_mask.offset += mask_offset;
392+
adjusted_mask.length = std::min(adjusted_mask.length - mask_offset, array.length);
393+
394+
RETURN_NOT_OK(VisitArraySpanInline<BooleanType>(
395+
adjusted_mask,
396+
[&](bool replace) -> Status {
397+
if (replace && replacements.is_scalar()) {
398+
// Scalar replacement: append the scalar value once.
399+
RETURN_NOT_OK(builder.AppendScalar(*replacements.scalar));
400+
} else {
401+
const ArraySpan& source = replace ? replacements.array : array;
402+
const int64_t offset = replace ? replacements_offset++ : source_offset;
403+
// Check validity of the source element at `offset`.
404+
const bool is_valid =
405+
!source.MayHaveNulls() ||
406+
bit_util::GetBit(source.buffers[0].data, source.offset + offset);
407+
if (is_valid) {
408+
// AppendArraySlice copies one list element (offset, 1) including its
409+
// child values and validity, correctly handling source.offset.
410+
RETURN_NOT_OK(builder.AppendArraySlice(source, offset, 1));
411+
} else {
412+
RETURN_NOT_OK(builder.AppendNull());
413+
}
414+
}
415+
source_offset++;
416+
return Status::OK();
417+
},
418+
[&]() -> Status {
419+
// Null mask entry → null output element.
420+
RETURN_NOT_OK(builder.AppendNull());
421+
source_offset++;
422+
return Status::OK();
423+
}));
424+
425+
std::shared_ptr<Array> temp_output;
426+
RETURN_NOT_OK(builder.Finish(&temp_output));
427+
out->value = std::move(temp_output->data());
428+
return replacements_offset;
429+
}
430+
};
431+
325432
Status CheckReplaceMaskInputs(const DataType& value_type, int64_t arr_length,
326433
const ExecValue& mask_box,
327434
const DataType& replacements_type,
@@ -862,8 +969,10 @@ void RegisterVectorFunction(FunctionRegistry* registry,
862969
GenerateTypeAgnosticVarBinaryBase<ChunkedFunctor>(*ty), registry,
863970
func.get());
864971
}
865-
// TODO: list types
866-
DCHECK_OK(registry->AddFunction(std::move(func)));
972+
// Note: list types (LIST, LARGE_LIST) are NOT added here. RegisterVectorFunction is
973+
// also used for fill_null_forward/fill_null_backward which do not yet support list
974+
// types. The caller is responsible for adding list kernels when appropriate and for
975+
// calling registry->AddFunction.
867976

868977
// TODO(ARROW-9431): "replace_with_indices"
869978
}
@@ -897,16 +1006,28 @@ void RegisterVectorReplace(FunctionRegistry* registry) {
8971006
auto func = std::make_shared<VectorFunction>("replace_with_mask", Arity::Ternary(),
8981007
replace_with_mask_doc);
8991008
RegisterVectorFunction<ReplaceMask, ReplaceMaskChunked>(registry, func);
1009+
// Add LIST and LARGE_LIST support. These are registered separately from
1010+
// RegisterVectorFunction because fill_null_forward/backward do not support list
1011+
// types yet; adding them here avoids requiring implementations for those kernels.
1012+
AddKernel(Type::LIST, ReplaceMask<ListType>::GetSignature(Type::LIST),
1013+
ReplaceMask<ListType>::Exec, ReplaceMaskChunked<ListType>::Exec, registry,
1014+
func.get());
1015+
AddKernel(Type::LARGE_LIST, ReplaceMask<LargeListType>::GetSignature(Type::LARGE_LIST),
1016+
ReplaceMask<LargeListType>::Exec, ReplaceMaskChunked<LargeListType>::Exec,
1017+
registry, func.get());
1018+
DCHECK_OK(registry->AddFunction(std::move(func)));
9001019
}
9011020
{
9021021
auto func = std::make_shared<VectorFunction>("fill_null_forward", Arity::Unary(),
9031022
fill_null_forward_doc);
9041023
RegisterVectorFunction<FillNullForward, FillNullForwardChunked>(registry, func);
1024+
DCHECK_OK(registry->AddFunction(std::move(func)));
9051025
}
9061026
{
9071027
auto func = std::make_shared<VectorFunction>("fill_null_backward", Arity::Unary(),
9081028
fill_null_backward_doc);
9091029
RegisterVectorFunction<FillNullBackward, FillNullBackwardChunked>(registry, func);
1030+
DCHECK_OK(registry->AddFunction(std::move(func)));
9101031
}
9111032
}
9121033
} // namespace internal

0 commit comments

Comments
 (0)