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]