Jay846 commented on code in PR #50976:
URL: https://github.com/apache/arrow/pull/50976#discussion_r3979534970
##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -3646,6 +3646,83 @@ TEST(Cast, ListToListOptionsPassthru) {
}
}
+TEST(Cast, ListViewToList) {
+ // 1. Contiguous ListView
+ auto contiguous_src = ArrayFromJSON(list_view(int16()), "[[10, 20], [30],
[40, 50]]");
+ auto contiguous_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30],
[40, 50]]");
+ CheckCast(contiguous_src, contiguous_expected);
+
+ // Assert zero-copy for contiguous values
+ ASSERT_OK_AND_ASSIGN(auto cast_result, Cast(contiguous_src, list(int16())));
+ auto src_lv = std::dynamic_pointer_cast<ListViewArray>(contiguous_src);
+ auto res_list =
std::dynamic_pointer_cast<ListArray>(cast_result.make_array());
+ ASSERT_EQ(res_list->values()->data()->buffers[1]->address(),
+ src_lv->values()->data()->buffers[1]->address());
+
+ // 2. Gapped/Non-contiguous ListView
+ auto values = ArrayFromJSON(int16(), "[10, 20, 999, 30, 40, 50]");
+ auto offsets = ArrayFromJSON(int32(), "[0, 3]");
+ auto sizes = ArrayFromJSON(int32(), "[2, 3]");
+ ASSERT_OK_AND_ASSIGN(auto gapped_src,
+ ListViewArray::FromArrays(*offsets, *sizes, *values));
+ auto gapped_expected = ArrayFromJSON(list(int16()), "[[10, 20], [30, 40,
50]]");
+ CheckCast(gapped_src, gapped_expected);
+
+ // 3. Overlapping ListView
+ auto overlapping_offsets = ArrayFromJSON(int32(), "[0, 1]");
+ auto overlapping_sizes = ArrayFromJSON(int32(), "[2, 2]");
+ ASSERT_OK_AND_ASSIGN(
+ auto overlapping_src,
+ ListViewArray::FromArrays(*overlapping_offsets, *overlapping_sizes,
*values));
+ auto overlapping_expected = ArrayFromJSON(list(int16()), "[[10, 20], [20,
999]]");
+ CheckCast(overlapping_src, overlapping_expected);
+
+ // 4. Large ListView to List and vice versa
+ auto large_contiguous_src =
+ ArrayFromJSON(large_list_view(int16()), "[[10, 20], [30], [40, 50]]");
+ auto large_contiguous_expected =
+ ArrayFromJSON(large_list(int16()), "[[10, 20], [30], [40, 50]]");
+ CheckCast(large_contiguous_src, large_contiguous_expected);
+ CheckCast(contiguous_src, large_contiguous_expected);
+ CheckCast(large_contiguous_src, contiguous_expected);
+
+ // 5. Null Propagation
+ auto nulls_src = ArrayFromJSON(list_view(int16()), "[[10, null], null, [40,
50]]");
+ auto nulls_expected = ArrayFromJSON(list(int16()), "[[10, null], null, [40,
50]]");
+ CheckCast(nulls_src, nulls_expected);
+
+ // 6. Generic and Nested Type casting
+ auto string_src =
+ ArrayFromJSON(list_view(utf8()), "[[\"a\", \"b\"], [\"c\"], [\"d\",
\"e\"]]");
+ auto string_expected =
+ ArrayFromJSON(list(utf8()), "[[\"a\", \"b\"], [\"c\"], [\"d\", \"e\"]]");
+ CheckCast(string_src, string_expected);
+
+ auto type_change_src = ArrayFromJSON(list_view(int16()), "[[10, 20], [30],
[40, 50]]");
+ auto type_change_expected = ArrayFromJSON(list(int32()), "[[10, 20], [30],
[40, 50]]");
+ CheckCast(type_change_src, type_change_expected);
+
+ // 7. Non-Contiguous Slice Boundary Verification
+ auto sliced_gapped_src = gapped_src->Slice(1, 1);
+ auto sliced_gapped_expected = ArrayFromJSON(list(int16()), "[[30, 40, 50]]");
+ CheckCast(sliced_gapped_src, sliced_gapped_expected);
+
+ auto sliced_overlapping_src = overlapping_src->Slice(1, 1);
+ auto sliced_overlapping_expected = ArrayFromJSON(list(int16()), "[[20,
999]]");
+ CheckCast(sliced_overlapping_src, sliced_overlapping_expected);
+
+ // 8. ListView with Nulls containing overflow values in null slots
+ auto null_val_src_values = ArrayFromJSON(int32(), "[10, 40000]");
+ auto null_val_src_offsets = ArrayFromJSON(int32(), "[0, 1]");
+ auto null_val_src_sizes = ArrayFromJSON(int32(), "[1, 1]");
+ ASSERT_OK_AND_ASSIGN(auto null_val_src, ListViewArray::FromArrays(
+ *null_val_src_offsets,
*null_val_src_sizes,
+ *null_val_src_values));
+ auto null_val_src_masked = MaskArrayWithNullsAt(null_val_src, {1});
+ auto null_val_expected = ArrayFromJSON(list(int16()), "[[10], null]");
+ CheckCast(null_val_src_masked, null_val_expected);
Review Comment:
Thanks for the detailed review @pitrou!
I'll address all the mandatory items:
- Replace the length == 0 check with DCHECK_NE and add zero-length test
coverage
- Add the template comment // (Large)ListView<T> -> (Large)List<U>
- Rename the struct to CastListViewToVarList
- Factor out the offset allocation above the if/else branch
- Simplify the end_offset calculation and the slice call
- Add ValidateFull and null entries to the tests
- Added null entries to the contiguous test examples
- Added a zero-length input test case
The SetBitRunReader optimization and unified loop are noted as future
improvements. Will push the fixes very shortly!
--
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]
