cloud-fan commented on code in PR #57210:
URL: https://github.com/apache/spark/pull/57210#discussion_r3584431212
##########
sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java:
##########
@@ -500,33 +507,59 @@ final UTF8String getUTF8String(int rowId) {
}
}
+ // Covers VarBinaryVector, LargeVarBinaryVector and ViewVarBinaryVector: all
of them return
+ // the value as a byte array from getObject, with a null check.
static class BinaryAccessor extends ArrowVectorAccessor {
- private final VarBinaryVector accessor;
+ private final VariableWidthFieldVector accessor;
- BinaryAccessor(VarBinaryVector vector) {
+ BinaryAccessor(VariableWidthFieldVector vector) {
super(vector);
this.accessor = vector;
}
@Override
final byte[] getBinary(int rowId) {
- return accessor.getObject(rowId);
+ return (byte[]) accessor.getObject(rowId);
}
}
- static class LargeBinaryAccessor extends ArrowVectorAccessor {
+ static class StringViewAccessor extends ArrowVectorAccessor {
- private final LargeVarBinaryVector accessor;
+ private final ViewVarCharVector accessor;
- LargeBinaryAccessor(LargeVarBinaryVector vector) {
+ StringViewAccessor(ViewVarCharVector vector) {
super(vector);
this.accessor = vector;
}
@Override
- final byte[] getBinary(int rowId) {
- return accessor.getObject(rowId);
+ final UTF8String getUTF8String(int rowId) {
+ if (accessor.isNull(rowId)) {
+ return null;
+ }
+ // Decode the 16-byte view struct directly rather than through Arrow's
+ // NullableViewVarCharHolder: the holder's int start/end fields truncate
the inline-value
+ // offset (rowId * 16 + 4) once the views buffer grows past
Integer.MAX_VALUE bytes, which
+ // would silently read the wrong memory. Both branches read the value
with zero copy, like
+ // the non-view string accessors.
+ ArrowBuf views = accessor.getDataBuffer();
+ long viewOffset = (long) rowId *
BaseVariableWidthViewVector.ELEMENT_SIZE;
+ int length = views.getInt(viewOffset);
+ if (length <= BaseVariableWidthViewVector.INLINE_SIZE) {
+ // Short values are stored inline in the views buffer, right after the
length.
+ long start = viewOffset + BaseVariableWidthViewVector.LENGTH_WIDTH;
+ return UTF8String.fromAddress(null, views.memoryAddress() + start,
length);
+ } else {
+ // Long values live in one of the variadic data buffers; the view
struct holds the buffer
+ // index and the offset within that buffer, after the length and a
4-byte prefix.
+ long bufferIndexOffset = viewOffset +
BaseVariableWidthViewVector.LENGTH_WIDTH
+ + BaseVariableWidthViewVector.PREFIX_WIDTH;
+ int bufferIndex = views.getInt(bufferIndexOffset);
+ int start = views.getInt(bufferIndexOffset +
BaseVariableWidthViewVector.BUF_INDEX_WIDTH);
+ ArrowBuf dataBuffer = accessor.getDataBuffers().get(bufferIndex);
Review Comment:
Non-blocking (test coverage): the long-value branch here indexes
`getDataBuffers().get(bufferIndex)`, but every long value in the
`string_view`/`binary_view`/deserialization tests is small enough to land in a
single variadic data buffer, so `bufferIndex` is always `0` — the multi-buffer
path is never exercised. Risk is low since the decode matches Arrow's own
`getData`, but a test with enough long values to spill into a second data
buffer would close the gap.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
