Jay846 commented on code in PR #50976:
URL: https://github.com/apache/arrow/pull/50976#discussion_r3861036964
##########
cpp/src/arrow/compute/kernels/scalar_cast_test.cc:
##########
@@ -3646,6 +3646,72 @@ TEST(Cast, ListToListOptionsPassthru) {
}
}
+TEST(Cast, ListViewToList) {
Review Comment:
You are completely right that the compute engine doesn't throw a
NotImplemented or unsupported type-casting error at runtime.
This is because the generic AddListCast<ListViewType, ListType> template
registration was already present in GetNestedCasts(), meaning Arrow incorrectly
routed the cast through the standard list-to-list kernel (CastList).
However, CastList is incorrect for ListView and leads to data corruption:
Ignores sizes buffer: Standard List assumes elements are contiguous (i.e.,
size[i] = offsets[i+1] - offsets[i]). For non-contiguous, gapped, or
overlapping ListView inputs, ignoring the sizes buffer yields a corrupted
ListArray referencing incorrect data slices.
Out-of-bounds offsets read: CastList attempts to read length + 1 offsets
from the source buffer. However, ListView only requires length offsets, leading
to an out-of-bounds read and corrupt/negative offset outputs.
If we run the newly added tests with the casting changes reverted, they will
fail during Google Test validation checks with errors such as:
Offset invariant failure: non-monotonic offset
Offsets buffer size (bytes) isn't large enough
The new CastListView functor introduced in this PR is necessary to support
zero-copy contiguous conversions and fallback paths (re-indexing via the take
kernel) to construct valid, spec-compliant ListArrays.
--
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]
