viirya commented on code in PR #967:
URL: https://github.com/apache/arrow-java/pull/967#discussion_r2710819297
##########
vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java:
##########
@@ -275,6 +275,17 @@ public void loadFieldBuffers(ArrowFieldNode fieldNode,
List<ArrowBuf> ownBuffers
@Override
public List<ArrowBuf> getFieldBuffers() {
List<ArrowBuf> result = new ArrayList<>(2);
+
+ // Ensure offset buffer has at least one entry for offset[0].
+ // According to Arrow specification, offset buffer must have N+1 entries,
+ // even when N=0, it should contain [0].
+ if (offsetBuffer.capacity() == 0) {
+ // Save and restore offsetAllocationSizeInBytes to avoid affecting
subsequent allocateNew()
+ long savedOffsetAllocationSize = offsetAllocationSizeInBytes;
+ offsetBuffer = allocateOffsetBuffer(OFFSET_WIDTH);
+ offsetAllocationSizeInBytes = savedOffsetAllocationSize;
+ }
Review Comment:
This looks unnecessary because once `allocateNew` is called on the vector,
its offset buffer should be allocated, and we shouldn't modify `offsetBuffer`
in a getter method like that.
##########
vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java:
##########
@@ -309,7 +320,8 @@ private void setReaderAndWriterIndex() {
offsetBuffer.readerIndex(0);
if (valueCount == 0) {
validityBuffer.writerIndex(0);
- offsetBuffer.writerIndex(0);
+ // Even when valueCount is 0, offset buffer should have offset[0] per
Arrow spec
+ offsetBuffer.writerIndex(Math.min(OFFSET_WIDTH,
offsetBuffer.capacity()));
Review Comment:
I think that we only need `offsetBuffer.writerIndex((valueCount + 1) *
OFFSET_WIDTH);` like below.
--
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]
