HuaHuaY commented on code in PR #50976:
URL: https://github.com/apache/arrow/pull/50976#discussion_r3864863598
##########
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 (is_downcast) {
Review Comment:
I think we can use `if constexpr` here.
##########
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 may have a mistake during the previous review. I am not sure whether this
`if` condition will always evaluate to `false` due to the check at
`cpp/src/arrow/compute/exec.cc:786`; perhaps we can assume here that
`in_array.length` is never 0. Let's wait for comments from a reviewer who is
more familiar with Arrow Compute.
##########
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 (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);
+ }
+ }
+
+ if (is_downcast) {
Review Comment:
Same here.
--
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]
