Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 66 additions & 35 deletions cpp/src/arrow/compute/kernels/scalar_cast_nested.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,38 +191,62 @@ struct CastVarToFixedList {
// Additionally, checks if the null slots are all the list size. Returns true
// if so, which can be used to optimize the cast.
static Result<bool> ValidateLengths(const ArraySpan& in_array, int32_t list_size) {
const auto* offsets = in_array.GetValues<src_offset_type>(1);
src_offset_type expected_offset = offsets[0] + list_size;
// If there are nulls, it's possible that the null slots are the correct
// size. That enables an optimization later in this function.
bool nulls_have_correct_length = true;
if (in_array.GetNullCount() > 0) {
for (int64_t i = 1; i <= in_array.length; ++i) {
if (in_array.IsNull(i - 1)) {
if (offsets[i] != expected_offset) {
nulls_have_correct_length = false;
if constexpr (is_list_view_type<SrcType>::value) {
const auto* offsets = in_array.GetValues<src_offset_type>(1);
const auto* sizes = in_array.GetValues<src_offset_type>(2);
bool can_use_slice = true;
src_offset_type expected_offset = in_array.length > 0 ? offsets[0] : 0;
for (int64_t i = 0; i < in_array.length; ++i) {
if (in_array.IsNull(i)) {
if (sizes[i] != list_size || offsets[i] != expected_offset) {
can_use_slice = false;
}
// If element is null, it can be any size, so the next offset is valid.
expected_offset = offsets[i] + list_size;
} else {
if (offsets[i] != expected_offset) {
if (sizes[i] != list_size) {
return Status::Invalid("ListType can only be casted to FixedSizeListType ",
"if the lists are all the expected size.");
}
expected_offset += list_size;
if (offsets[i] != expected_offset) {
can_use_slice = false;
}
}
expected_offset += list_size;
}
return can_use_slice;
} else {
// Don't need to check null slots if there are no nulls
for (int64_t i = 1; i <= in_array.length; ++i) {
if (offsets[i] != expected_offset) {
return Status::Invalid("ListType can only be casted to FixedSizeListType ",
"if the lists are all the expected size.");
const auto* offsets = in_array.GetValues<src_offset_type>(1);
src_offset_type expected_offset = offsets[0] + list_size;
// If there are nulls, it's possible that the null slots are the correct
// size. That enables an optimization later in this function.
bool nulls_have_correct_length = true;
if (in_array.GetNullCount() > 0) {
for (int64_t i = 1; i <= in_array.length; ++i) {
if (in_array.IsNull(i - 1)) {
if (offsets[i] != expected_offset) {
nulls_have_correct_length = false;
}
// If element is null, it can be any size, so the next offset is valid.
expected_offset = offsets[i] + list_size;
} else {
if (offsets[i] != expected_offset) {
return Status::Invalid("ListType can only be casted to FixedSizeListType ",
"if the lists are all the expected size.");
}
expected_offset += list_size;
}
}
} else {
// Don't need to check null slots if there are no nulls
for (int64_t i = 1; i <= in_array.length; ++i) {
if (offsets[i] != expected_offset) {
return Status::Invalid("ListType can only be casted to FixedSizeListType ",
"if the lists are all the expected size.");
}
expected_offset += list_size;
}
expected_offset += list_size;
}
return nulls_have_correct_length;
}
return nulls_have_correct_length;
}

// Build take indices for the values. This is used to fill in the null slots
Expand All @@ -236,9 +260,9 @@ struct CastVarToFixedList {
RETURN_NOT_OK(builder.Reserve(in_array.length * list_size));
for (int64_t offset_i = 0; offset_i < in_array.length; ++offset_i) {
if (in_array.IsNull(offset_i)) {
// If element is null, just fill in the null slots with first value.
// If element is null, fill in the null slots with null values.
for (int64_t j = 0; j < list_size; ++j) {
builder.UnsafeAppend(0);
builder.UnsafeAppendNull();
}
} else {
int64_t value_i = offsets[offset_i];
Expand Down Expand Up @@ -275,18 +299,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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ARROW_ASSIGN_OR_RAISE(
auto null_values,
MakeArrayOfNull(child_type, in_array.length * list_size, ctx->memory_pool()));
cast_values = null_values->data();
} else {
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 || cast_values->length > in_array.length * list_size) {
// No nulls or nulls have correct length, but we need to slice the values
cast_values = cast_values->Slice(offsets[0], in_array.length * list_size);
}
}

out_array->child_data.emplace_back(std::move(cast_values));
Expand Down
97 changes: 97 additions & 0 deletions cpp/src/arrow/compute/kernels/scalar_cast_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3787,6 +3787,103 @@ 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(), "[]");
auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0]"), *child,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]");
CheckCast(from, to);
}

// GH-54570: LargeList with empty child cast to FixedSizeList
{
auto child = ArrayFromJSON(int64(), "[]");
auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
LargeListArray::FromArrays(*ArrayFromJSON(int64(), "[0, 0]"), *child,
default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]");
CheckCast(from, to);
}

// GH-54570: All-null ListView with empty child cast to FixedSizeList
{
auto child = ArrayFromJSON(int64(), "[]");
auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
ListViewArray::FromArrays(*ArrayFromJSON(int32(), "[0]"),
*ArrayFromJSON(int32(), "[0]"), *child,
default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]");
CheckCast(from, to);
}

// GH-54570: All-null LargeListView with empty child cast to FixedSizeList
{
auto child = ArrayFromJSON(int64(), "[]");
auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
LargeListViewArray::FromArrays(*ArrayFromJSON(int64(), "[0]"),
*ArrayFromJSON(int64(), "[0]"), *child,
default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]");
CheckCast(from, to);
}

// GH-54570: Multiple null lists with empty child
{
auto child = ArrayFromJSON(int64(), "[]");
auto null_bitmap = ArrayFromJSON(boolean(), "[false, false]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0, 0]"), *child,
default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int64(), 2), "[null, null]");
CheckCast(from, to);
}

// GH-54570: Null list with unallocated/null child data buffers
{
auto child_data = ArrayData::Make(int64(), 0, {nullptr, nullptr});
auto list_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1];
auto list_offsets = ArrayFromJSON(int32(), "[0, 0]")->data()->buffers[1];
auto list_data = ArrayData::Make(list(int64()), 1, {list_bitmap, list_offsets},
{child_data}, 1);
auto from = MakeArray(list_data);
auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]");
CheckCast(from, to);
}

// GH-54570: Null slots with length 0 alongside valid slots
{
auto child = ArrayFromJSON(int32(), "[10, 20]");
auto null_bitmap = ArrayFromJSON(boolean(), "[false, true]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0, 2]"), *child,
default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[null, [10, 20]]");
CheckCast(from, to);
}

{
auto child = ArrayFromJSON(int32(), "[10, 20]");
auto null_bitmap = ArrayFromJSON(boolean(), "[true, false]")->data()->buffers[1];
ASSERT_OK_AND_ASSIGN(
auto from,
ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 2, 2]"), *child,
default_memory_pool(), null_bitmap));
auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[[10, 20], null]");
CheckCast(from, to);
}
}

TEST(Cast, CastMap) {
Expand Down
Loading