lidavidm commented on code in PR #14181:
URL: https://github.com/apache/arrow/pull/14181#discussion_r978007207


##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -140,6 +140,38 @@ void AddListCast(CastFunction* func) {
   DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
 }
 
+struct CastFixedList {
+  static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult* 
out) {
+    const CastOptions& options = CastState::Get(ctx);
+    const auto& in_type = checked_cast<const 
FixedSizeListType&>(*batch[0].type());
+    const auto& out_type = checked_cast<const 
FixedSizeListType&>(*out->type());
+    auto in_size = in_type.list_size();
+    auto out_size = out_type.list_size();
+
+    if (in_size != out_size) {
+      return Status::TypeError("Size of FixedSizeList is not the same.",
+                               " input list: ", in_type.ToString(),
+                               " output list: ", out_type.ToString());
+    }
+
+    const ArraySpan& in_array = batch[0].array;
+    std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+    // Take care of data if input are is a view.
+    values = values->Slice(in_array.offset * in_size, in_array.length * 
in_size);

Review Comment:
   This needs to take into account the child array's offset, too



##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -2209,6 +2209,45 @@ TEST(Cast, ListToListOptionsPassthru) {
   }
 }
 
+static void CheckFSLToFSL(const std::vector<std::shared_ptr<DataType>>& 
value_types,
+                          const std::string& json_data) {
+  for (const auto& src_value_type : value_types) {
+    for (const auto& dest_value_type : value_types) {
+      const auto src_type = fixed_size_list(src_value_type, 2);
+      const auto dest_type = fixed_size_list(dest_value_type, 2);
+      ARROW_SCOPED_TRACE("src_type = ", src_type->ToString(),
+                         ", dest_type = ", dest_type->ToString());
+      auto src_array = ArrayFromJSON(src_type, json_data);
+      CheckCast(src_array, ArrayFromJSON(dest_type, json_data));

Review Comment:
   I guess we should also test casting slices of the input and comparing it to 
slices of the output here. 



##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -2209,6 +2209,39 @@ TEST(Cast, ListToListOptionsPassthru) {
   }
 }
 
+static void CheckFSLToFSL(const std::vector<std::shared_ptr<DataType>>& 
value_types,
+                          const std::string& json_data) {
+  for (const auto& src_value_type : value_types) {
+    for (const auto& dest_value_type : value_types) {
+      const auto src_type = fixed_size_list(src_value_type, 2);
+      const auto dest_type = fixed_size_list(dest_value_type, 2);
+      ARROW_SCOPED_TRACE("src_type = ", src_type->ToString(),
+                         ", dest_type = ", dest_type->ToString());
+      auto src_array = ArrayFromJSON(src_type, json_data);
+      auto dst_array = ArrayFromJSON(dest_type, json_data);
+      CheckCast(src_array, dst_array);
+    }
+  }
+}
+
+TEST(Cast, FSLToFSL) {
+  CheckFSLToFSL({int32(), float32(), int64()}, "[[0, 1], [2, 3], [null, 5]]");

Review Comment:
   Hmm, in that case, we should probably check what happens when the child 
array also has an offset…



##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -2209,6 +2209,45 @@ TEST(Cast, ListToListOptionsPassthru) {
   }
 }
 
+static void CheckFSLToFSL(const std::vector<std::shared_ptr<DataType>>& 
value_types,
+                          const std::string& json_data) {
+  for (const auto& src_value_type : value_types) {
+    for (const auto& dest_value_type : value_types) {
+      const auto src_type = fixed_size_list(src_value_type, 2);
+      const auto dest_type = fixed_size_list(dest_value_type, 2);
+      ARROW_SCOPED_TRACE("src_type = ", src_type->ToString(),
+                         ", dest_type = ", dest_type->ToString());
+      auto src_array = ArrayFromJSON(src_type, json_data);
+      CheckCast(src_array, ArrayFromJSON(dest_type, json_data));

Review Comment:
   We should also slice the child array, construct the parent array (with 
nulls, etc.) and test that as well. That should probably be a separate 
test/helper function. There's a helper function called TweakValidityBit that is 
useful for these sorts of things



##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -2209,6 +2209,39 @@ TEST(Cast, ListToListOptionsPassthru) {
   }
 }
 
+static void CheckFSLToFSL(const std::vector<std::shared_ptr<DataType>>& 
value_types,
+                          const std::string& json_data) {
+  for (const auto& src_value_type : value_types) {
+    for (const auto& dest_value_type : value_types) {
+      const auto src_type = fixed_size_list(src_value_type, 2);
+      const auto dest_type = fixed_size_list(dest_value_type, 2);
+      ARROW_SCOPED_TRACE("src_type = ", src_type->ToString(),
+                         ", dest_type = ", dest_type->ToString());
+      auto src_array = ArrayFromJSON(src_type, json_data);
+      auto dst_array = ArrayFromJSON(dest_type, json_data);
+      CheckCast(src_array, dst_array);
+    }
+  }
+}
+
+TEST(Cast, FSLToFSL) {
+  CheckFSLToFSL({int32(), float32(), int64()}, "[[0, 1], [2, 3], [null, 5]]");

Review Comment:
   Left comments above. We should test for offsets here too.



##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -140,6 +140,38 @@ void AddListCast(CastFunction* func) {
   DCHECK_OK(func->AddKernel(SrcType::type_id, std::move(kernel)));
 }
 
+struct CastFixedList {
+  static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult* 
out) {
+    const CastOptions& options = CastState::Get(ctx);
+    const auto& in_type = checked_cast<const 
FixedSizeListType&>(*batch[0].type());
+    const auto& out_type = checked_cast<const 
FixedSizeListType&>(*out->type());
+    auto in_size = in_type.list_size();
+    auto out_size = out_type.list_size();
+
+    if (in_size != out_size) {
+      return Status::TypeError("Size of FixedSizeList is not the same.",
+                               " input list: ", in_type.ToString(),
+                               " output list: ", out_type.ToString());
+    }
+
+    const ArraySpan& in_array = batch[0].array;
+    std::shared_ptr<ArrayData> values = in_array.child_data[0].ToArrayData();
+    // Take care of data if input are is a view.
+    values = values->Slice(in_array.offset * in_size, in_array.length * 
in_size);
+
+    ArrayData* out_array = out->array_data().get();
+    out_array->buffers[0] = in_array.GetBuffer(0);

Review Comment:
   This needs to be adjusted according to the input array's offset



-- 
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]

Reply via email to