dmitry-chirkov-dremio commented on code in PR #54574:
URL: https://github.com/apache/arrow/pull/54574#discussion_r4335850544


##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -3787,6 +3787,72 @@ TEST(Cast, ListToFSL) {
   // Invalid fixed_size_list cast if inconsistent size
   ASSERT_RAISES(Invalid, Cast(ArrayFromJSON(list(int32()), "[[0, 1, 2], null, 
[3, 4]]"),
                               CastOptions::Safe(fixed_size_list(int32(), 3))));
+
+  // GH-54570: Null list array with empty child array cast to FixedSizeList
+  {
+    auto child = ArrayFromJSON(int64(), "[]");
+    ASSERT_OK_AND_ASSIGN(
+        auto from,
+        ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0]"), *child,

Review Comment:
   The C++ build has not run yet however compiling this locally reports an 
error because the third argument to FromArrays expects MemoryPool*, but 
receives an Array. 
   
   Did you compile/run this before submitting?



##########
cpp/src/arrow/compute/kernels/scalar_cast_nested.cc:
##########
@@ -275,18 +275,25 @@ struct CastVarToFixedList {
     DCHECK(cast_values_datum.is_array());
     std::shared_ptr<ArrayData> cast_values = cast_values_datum.array();
 
-    const auto* offsets = in_array.GetValues<src_offset_type>(1);
-    if (in_array.GetNullCount() > 0 && !nulls_have_correct_length) {
-      // We need to fill in the null slots, so we'll use Take on the values.
-      ARROW_ASSIGN_OR_RAISE(auto indices, BuildTakeIndices(in_array, 
list_size, ctx));
-      ARROW_ASSIGN_OR_RAISE(auto take_result,
-                            Take(cast_values, Datum(indices),
-                                 TakeOptions::NoBoundsCheck(), 
ctx->exec_context()));
-      DCHECK(take_result.is_array());
-      cast_values = take_result.array();
-    } else if (offsets[0] != 0) {
-      // No nulls, but we need to slice the values
-      cast_values = cast_values->Slice(offsets[0], in_array.length * 
list_size);
+    if (cast_values->length == 0) {

Review Comment:
   The list-view inputs mentioned in the description still fail before reaching 
this branch. ValidateLengths reads offsets[length], but list views have only 
length offsets and a separate sizes buffer.
   
   Please add regression tests for all-null ListView and LargeListView inputs 
with empty children. Either handle these layouts here or narrow the claimed 
scope and track the remaining bug separately.



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