pitrou commented on code in PR #50976:
URL: https://github.com/apache/arrow/pull/50976#discussion_r3979241437
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
+ if (in_array.IsNull(i) && sizes[i] != 0) {
+ return false;
+ }
+ if (offsets[i] + sizes[i] != offsets[i + 1]) {
Review Comment:
If the entry is null or zero-sized, then the exact value of `offsets[i]`
shouldn't matter and we can instead keep the value of the last non-null
non-zero entry?
(this is not a bug of course, just an additional optimization opportunity)
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
Review Comment:
Let's add a comment summarizing this:
```suggestion
// (Large)ListView<T> -> (Large)List<U>
template <typename SrcType, typename DestType>
```
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
Review Comment:
You could perhaps use `SetBitRunReader` to speed up walking the validity
bitmap (individual `IsNull` calls are more expensive), though that's not
necessary either.
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
+ if (in_array.IsNull(i) && sizes[i] != 0) {
+ return false;
+ }
+ if (offsets[i] + sizes[i] != offsets[i + 1]) {
+ return false;
+ }
+ }
+ if (in_array.length > 0 && in_array.IsNull(in_array.length - 1) &&
+ sizes[in_array.length - 1] != 0) {
+ return false;
+ }
+ return true;
+ }
+
+ static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
+ const CastOptions& options = CastState::Get(ctx);
+ auto child_type = checked_cast<const DestType&>(*out->type()).value_type();
+ const ArraySpan& in_array = batch[0].array;
+ ArrayData* out_array = out->array_data().get();
+
+ if (in_array.length == 0) {
+ out_array->buffers[0] = nullptr;
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+ dest_offsets[0] = 0;
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+ ARROW_ASSIGN_OR_RAISE(Datum cast_values, Cast(values->Slice(0, 0),
child_type,
+ options,
ctx->exec_context()));
+ DCHECK(cast_values.is_array());
+ out_array->child_data.push_back(cast_values.array());
+ return Status::OK();
+ }
+
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[0],
+ GetOrCopyNullBitmapBuffer(in_array,
ctx->memory_pool()));
+
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+
+ if (IsContiguous(in_array)) {
+ // Zero-copy fast-path: shift offsets and slice child values
+ ARROW_ASSIGN_OR_RAISE(
+ out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type) * (in_array.length + 1)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
Review Comment:
This is the same as below and can be factored out of the if/else branch.
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
+ if (in_array.IsNull(i) && sizes[i] != 0) {
+ return false;
+ }
+ if (offsets[i] + sizes[i] != offsets[i + 1]) {
+ return false;
+ }
+ }
+ if (in_array.length > 0 && in_array.IsNull(in_array.length - 1) &&
+ sizes[in_array.length - 1] != 0) {
+ return false;
+ }
+ return true;
+ }
+
+ static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
+ const CastOptions& options = CastState::Get(ctx);
+ auto child_type = checked_cast<const DestType&>(*out->type()).value_type();
+ const ArraySpan& in_array = batch[0].array;
+ ArrayData* out_array = out->array_data().get();
+
+ if (in_array.length == 0) {
+ out_array->buffers[0] = nullptr;
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+ dest_offsets[0] = 0;
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+ ARROW_ASSIGN_OR_RAISE(Datum cast_values, Cast(values->Slice(0, 0),
child_type,
+ options,
ctx->exec_context()));
+ DCHECK(cast_values.is_array());
+ out_array->child_data.push_back(cast_values.array());
+ return Status::OK();
+ }
+
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[0],
+ GetOrCopyNullBitmapBuffer(in_array,
ctx->memory_pool()));
+
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+
+ if (IsContiguous(in_array)) {
+ // Zero-copy fast-path: shift offsets and slice child values
+ ARROW_ASSIGN_OR_RAISE(
+ out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type) * (in_array.length + 1)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+
+ src_offset_type start_offset = offsets[0];
+ src_offset_type end_offset =
+ offsets[in_array.length - 1] + sizes[in_array.length - 1] -
start_offset;
+
+ if constexpr (is_downcast) {
+ if (end_offset > std::numeric_limits<dest_offset_type>::max()) {
+ return Status::Invalid("Array of type ", in_array.type->ToString(),
+ " too large to convert to ",
+ out_array->type->ToString());
+ }
+ }
+
+ for (int64_t i = 0; i < in_array.length; ++i) {
+ dest_offsets[i] = static_cast<dest_offset_type>(offsets[i] -
start_offset);
+ }
+ dest_offsets[in_array.length] =
static_cast<dest_offset_type>(end_offset);
+
+ values = values->Slice(start_offset, dest_offsets[in_array.length]);
+ } else {
+ // Non-contiguous path: compute new offsets, flatten/concatenate values
+ ARROW_ASSIGN_OR_RAISE(
+ out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type) * (in_array.length + 1)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+
+ src_offset_type current_offset = 0;
+ dest_offsets[0] = 0;
+ for (int64_t i = 0; i < in_array.length; ++i) {
+ if (in_array.IsNull(i)) {
+ dest_offsets[i + 1] = static_cast<dest_offset_type>(current_offset);
+ } else {
+ current_offset += sizes[i];
+ dest_offsets[i + 1] = static_cast<dest_offset_type>(current_offset);
+ }
+ }
Review Comment:
I think you could simplify the implementation by having the same loop
offsets for both branches. That loop would compute all destination offsets
*and* compute whether the source entries are contiguous, all in one go.
The contiguity information is mostly useful to know how to compute `values`
afterwards. It needn't affect the computation of destination offsets, which has
roughly the same costs in both cases.
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
+ if (in_array.IsNull(i) && sizes[i] != 0) {
+ return false;
+ }
+ if (offsets[i] + sizes[i] != offsets[i + 1]) {
+ return false;
+ }
+ }
+ if (in_array.length > 0 && in_array.IsNull(in_array.length - 1) &&
+ sizes[in_array.length - 1] != 0) {
+ return false;
+ }
+ return true;
+ }
+
+ static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
+ const CastOptions& options = CastState::Get(ctx);
+ auto child_type = checked_cast<const DestType&>(*out->type()).value_type();
+ const ArraySpan& in_array = batch[0].array;
+ ArrayData* out_array = out->array_data().get();
+
+ if (in_array.length == 0) {
+ out_array->buffers[0] = nullptr;
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+ dest_offsets[0] = 0;
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+ ARROW_ASSIGN_OR_RAISE(Datum cast_values, Cast(values->Slice(0, 0),
child_type,
+ options,
ctx->exec_context()));
+ DCHECK(cast_values.is_array());
+ out_array->child_data.push_back(cast_values.array());
+ return Status::OK();
+ }
+
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[0],
+ GetOrCopyNullBitmapBuffer(in_array,
ctx->memory_pool()));
+
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+
+ if (IsContiguous(in_array)) {
+ // Zero-copy fast-path: shift offsets and slice child values
+ ARROW_ASSIGN_OR_RAISE(
+ out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type) * (in_array.length + 1)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+
+ src_offset_type start_offset = offsets[0];
+ src_offset_type end_offset =
+ offsets[in_array.length - 1] + sizes[in_array.length - 1] -
start_offset;
+
+ if constexpr (is_downcast) {
+ if (end_offset > std::numeric_limits<dest_offset_type>::max()) {
+ return Status::Invalid("Array of type ", in_array.type->ToString(),
+ " too large to convert to ",
+ out_array->type->ToString());
+ }
+ }
+
+ for (int64_t i = 0; i < in_array.length; ++i) {
+ dest_offsets[i] = static_cast<dest_offset_type>(offsets[i] -
start_offset);
+ }
+ dest_offsets[in_array.length] =
static_cast<dest_offset_type>(end_offset);
+
+ values = values->Slice(start_offset, dest_offsets[in_array.length]);
Review Comment:
`dest_offsets[in_array.length]` is just `end_offset` here.
##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -3646,6 +3646,83 @@ TEST(Cast, ListToListOptionsPassthru) {
}
}
+TEST(Cast, ListViewToList) {
+ // 1. Contiguous ListView
+ auto contiguous_src = ArrayFromJSON(list_view(int16()), "[[10, 20], [30],
[40, 50]]");
+ auto contiguous_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30],
[40, 50]]");
+ CheckCast(contiguous_src, contiguous_expected);
+
+ // Assert zero-copy for contiguous values
+ ASSERT_OK_AND_ASSIGN(auto cast_result, Cast(contiguous_src, list(int16())));
+ auto src_lv = std::dynamic_pointer_cast<ListViewArray>(contiguous_src);
+ auto res_list =
std::dynamic_pointer_cast<ListArray>(cast_result.make_array());
+ ASSERT_EQ(res_list->values()->data()->buffers[1]->address(),
+ src_lv->values()->data()->buffers[1]->address());
+
+ // 2. Gapped/Non-contiguous ListView
+ auto values = ArrayFromJSON(int16(), "[10, 20, 999, 30, 40, 50]");
+ auto offsets = ArrayFromJSON(int32(), "[0, 3]");
+ auto sizes = ArrayFromJSON(int32(), "[2, 3]");
+ ASSERT_OK_AND_ASSIGN(auto gapped_src,
+ ListViewArray::FromArrays(*offsets, *sizes, *values));
+ auto gapped_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30, 40,
50]]");
+ CheckCast(gapped_src, gapped_expected);
+
+ // 3. Overlapping ListView
+ auto overlapping_offsets = ArrayFromJSON(int32(), "[0, 1]");
+ auto overlapping_sizes = ArrayFromJSON(int32(), "[2, 2]");
+ ASSERT_OK_AND_ASSIGN(
+ auto overlapping_src,
+ ListViewArray::FromArrays(*overlapping_offsets, *overlapping_sizes,
*values));
+ auto overlapping_expected = ArrayFromJSON(list(int16()), "[[10, 20], [20,
999]]");
+ CheckCast(overlapping_src, overlapping_expected);
+
+ // 4. Large ListView to List and vice versa
+ auto large_contiguous_src =
+ ArrayFromJSON(large_list_view(int16()), "[[10, 20], [30], [40, 50]]");
+ auto large_contiguous_expected =
+ ArrayFromJSON(large_list(int16()), "[[10, 20], [30], [40, 50]]");
+ CheckCast(large_contiguous_src, large_contiguous_expected);
+ CheckCast(contiguous_src, large_contiguous_expected);
+ CheckCast(large_contiguous_src, contiguous_expected);
+
+ // 5. Null Propagation
Review Comment:
Can you add nulls in the examples above? This will probably stress more
situations.
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
Review Comment:
Call this `CallListViewToVarList` to make sure it's not used for casting to
another list-view type?
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
+ if (in_array.IsNull(i) && sizes[i] != 0) {
+ return false;
+ }
+ if (offsets[i] + sizes[i] != offsets[i + 1]) {
+ return false;
+ }
+ }
+ if (in_array.length > 0 && in_array.IsNull(in_array.length - 1) &&
+ sizes[in_array.length - 1] != 0) {
+ return false;
+ }
+ return true;
+ }
+
+ static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
+ const CastOptions& options = CastState::Get(ctx);
+ auto child_type = checked_cast<const DestType&>(*out->type()).value_type();
+ const ArraySpan& in_array = batch[0].array;
+ ArrayData* out_array = out->array_data().get();
+
+ if (in_array.length == 0) {
+ out_array->buffers[0] = nullptr;
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+ dest_offsets[0] = 0;
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+ ARROW_ASSIGN_OR_RAISE(Datum cast_values, Cast(values->Slice(0, 0),
child_type,
+ options,
ctx->exec_context()));
+ DCHECK(cast_values.is_array());
+ out_array->child_data.push_back(cast_values.array());
+ return Status::OK();
+ }
+
+ ARROW_ASSIGN_OR_RAISE(out_array->buffers[0],
+ GetOrCopyNullBitmapBuffer(in_array,
ctx->memory_pool()));
+
+ std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+
+ if (IsContiguous(in_array)) {
+ // Zero-copy fast-path: shift offsets and slice child values
+ ARROW_ASSIGN_OR_RAISE(
+ out_array->buffers[1],
+ ctx->Allocate(sizeof(dest_offset_type) * (in_array.length + 1)));
+ auto* dest_offsets = out_array->GetMutableValues<dest_offset_type>(1);
+
+ src_offset_type start_offset = offsets[0];
+ src_offset_type end_offset =
+ offsets[in_array.length - 1] + sizes[in_array.length - 1] -
start_offset;
Review Comment:
Basing `end_offset` off of `start_offset` makes the code more confusing to
read than it should, IMHO.
##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -3646,6 +3646,83 @@ TEST(Cast, ListToListOptionsPassthru) {
}
}
+TEST(Cast, ListViewToList) {
+ // 1. Contiguous ListView
+ auto contiguous_src = ArrayFromJSON(list_view(int16()), "[[10, 20], [30],
[40, 50]]");
+ auto contiguous_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30],
[40, 50]]");
+ CheckCast(contiguous_src, contiguous_expected);
+
+ // Assert zero-copy for contiguous values
+ ASSERT_OK_AND_ASSIGN(auto cast_result, Cast(contiguous_src, list(int16())));
+ auto src_lv = std::dynamic_pointer_cast<ListViewArray>(contiguous_src);
+ auto res_list =
std::dynamic_pointer_cast<ListArray>(cast_result.make_array());
+ ASSERT_EQ(res_list->values()->data()->buffers[1]->address(),
+ src_lv->values()->data()->buffers[1]->address());
+
+ // 2. Gapped/Non-contiguous ListView
+ auto values = ArrayFromJSON(int16(), "[10, 20, 999, 30, 40, 50]");
+ auto offsets = ArrayFromJSON(int32(), "[0, 3]");
+ auto sizes = ArrayFromJSON(int32(), "[2, 3]");
+ ASSERT_OK_AND_ASSIGN(auto gapped_src,
+ ListViewArray::FromArrays(*offsets, *sizes, *values));
+ auto gapped_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30, 40,
50]]");
+ CheckCast(gapped_src, gapped_expected);
+
+ // 3. Overlapping ListView
+ auto overlapping_offsets = ArrayFromJSON(int32(), "[0, 1]");
+ auto overlapping_sizes = ArrayFromJSON(int32(), "[2, 2]");
+ ASSERT_OK_AND_ASSIGN(
+ auto overlapping_src,
+ ListViewArray::FromArrays(*overlapping_offsets, *overlapping_sizes,
*values));
+ auto overlapping_expected = ArrayFromJSON(list(int16()), "[[10, 20], [20,
999]]");
+ CheckCast(overlapping_src, overlapping_expected);
+
+ // 4. Large ListView to List and vice versa
+ auto large_contiguous_src =
+ ArrayFromJSON(large_list_view(int16()), "[[10, 20], [30], [40, 50]]");
+ auto large_contiguous_expected =
+ ArrayFromJSON(large_list(int16()), "[[10, 20], [30], [40, 50]]");
+ CheckCast(large_contiguous_src, large_contiguous_expected);
+ CheckCast(contiguous_src, large_contiguous_expected);
+ CheckCast(large_contiguous_src, contiguous_expected);
+
+ // 5. Null Propagation
+ auto nulls_src = ArrayFromJSON(list_view(int16()), "[[10, null], null, [40,
50]]");
+ auto nulls_expected = ArrayFromJSON(list(int16()), "[[10, null], null, [40,
50]]");
+ CheckCast(nulls_src, nulls_expected);
+
+ // 6. Generic and Nested Type casting
+ auto string_src =
+ ArrayFromJSON(list_view(utf8()), "[[\"a\", \"b\"], [\"c\"], [\"d\",
\"e\"]]");
+ auto string_expected =
+ ArrayFromJSON(list(utf8()), "[[\"a\", \"b\"], [\"c\"], [\"d\", \"e\"]]");
+ CheckCast(string_src, string_expected);
+
+ auto type_change_src = ArrayFromJSON(list_view(int16()), "[[10, 20], [30],
[40, 50]]");
+ auto type_change_expected = ArrayFromJSON(list(int32()), "[[10, 20], [30],
[40, 50]]");
+ CheckCast(type_change_src, type_change_expected);
+
+ // 7. Non-Contiguous Slice Boundary Verification
+ auto sliced_gapped_src = gapped_src->Slice(1, 1);
+ auto sliced_gapped_expected = ArrayFromJSON(list(int16()), "[[30, 40, 50]]");
+ CheckCast(sliced_gapped_src, sliced_gapped_expected);
+
+ auto sliced_overlapping_src = overlapping_src->Slice(1, 1);
+ auto sliced_overlapping_expected = ArrayFromJSON(list(int16()), "[[20,
999]]");
+ CheckCast(sliced_overlapping_src, sliced_overlapping_expected);
+
+ // 8. ListView with Nulls containing overflow values in null slots
+ auto null_val_src_values = ArrayFromJSON(int32(), "[10, 40000]");
+ auto null_val_src_offsets = ArrayFromJSON(int32(), "[0, 1]");
+ auto null_val_src_sizes = ArrayFromJSON(int32(), "[1, 1]");
+ ASSERT_OK_AND_ASSIGN(auto null_val_src, ListViewArray::FromArrays(
+ *null_val_src_offsets,
*null_val_src_sizes,
+ *null_val_src_values));
+ auto null_val_src_masked = MaskArrayWithNullsAt(null_val_src, {1});
+ auto null_val_expected = ArrayFromJSON(list(int16()), "[[10], null]");
+ CheckCast(null_val_src_masked, null_val_expected);
Review Comment:
Can you add a test with zero-length inputs?
##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -3646,6 +3646,83 @@ TEST(Cast, ListToListOptionsPassthru) {
}
}
+TEST(Cast, ListViewToList) {
+ // 1. Contiguous ListView
+ auto contiguous_src = ArrayFromJSON(list_view(int16()), "[[10, 20], [30],
[40, 50]]");
+ auto contiguous_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30],
[40, 50]]");
+ CheckCast(contiguous_src, contiguous_expected);
+
+ // Assert zero-copy for contiguous values
+ ASSERT_OK_AND_ASSIGN(auto cast_result, Cast(contiguous_src, list(int16())));
Review Comment:
Can you call `ValidateFull` on the cast result?
##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -141,6 +142,138 @@ void AddListCast(CastFunction* func) {
DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
}
+template <typename SrcType, typename DestType>
+struct CastListView {
+ using src_offset_type = typename SrcType::offset_type;
+ using dest_offset_type = typename DestType::offset_type;
+
+ static constexpr bool is_downcast = sizeof(src_offset_type) >
sizeof(dest_offset_type);
+
+ static bool IsContiguous(const ArraySpan& in_array) {
+ const auto* offsets = in_array.GetValues<src_offset_type>(1);
+ const auto* sizes = in_array.GetValues<src_offset_type>(2);
+ for (int64_t i = 0; i < in_array.length - 1; ++i) {
+ if (in_array.IsNull(i) && sizes[i] != 0) {
+ return false;
+ }
+ if (offsets[i] + sizes[i] != offsets[i + 1]) {
+ return false;
+ }
+ }
+ if (in_array.length > 0 && in_array.IsNull(in_array.length - 1) &&
+ sizes[in_array.length - 1] != 0) {
+ return false;
+ }
+ return true;
+ }
+
+ static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
+ const CastOptions& options = CastState::Get(ctx);
+ auto child_type = checked_cast<const DestType&>(*out->type()).value_type();
+ const ArraySpan& in_array = batch[0].array;
+ ArrayData* out_array = out->array_data().get();
+
+ if (in_array.length == 0) {
Review Comment:
I think we can replace this with a `DCHECK_NE(in_array.length, 0)` and
ensure that the tests exercise zero-length arrays and chunked arrays.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
