This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-java.git
The following commit(s) were added to refs/heads/main by this push:
new 8ce39730f GH-1179: Correct the size of var-width vector with >0 start
offset during vector append (#1180)
8ce39730f is described below
commit 8ce39730fe969b41c8620b02f3fa77c974d90784
Author: Jordan Epstein <[email protected]>
AuthorDate: Thu Jul 2 18:16:18 2026 -0500
GH-1179: Correct the size of var-width vector with >0 start offset during
vector append (#1180)
## What's Changed
Fix VectorAppender data size computation for variable-width vectors with
non-zero start offsets
When appending a variable width offset vector in DataFusion comet I was
receiving exceptions
due to allocating too much memory. This is because Comet passes variable
width arrays back
to Java where the initial offset vector entry is greater than 0. Prior
to this change, arrow-java
determines how many bytes to copy by just looking at the last offset
entry in the buffer,
completely disregarding the value of the first. If first = 100 and last
= 200, Java will still
copy 200 bytes instead of 100. In this change we fix that.
Closes #1179
---------
Co-authored-by: Jordan Epstein <[email protected]>
---
.../apache/arrow/vector/util/VectorAppender.java | 92 +++++++---
.../arrow/vector/util/TestVectorAppender.java | 189 +++++++++++++++++++++
2 files changed, 257 insertions(+), 24 deletions(-)
diff --git
a/vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java
b/vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java
index e7c0d11cb..2cfeb0a04 100644
--- a/vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java
+++ b/vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java
@@ -125,10 +125,15 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
targetVector
.getOffsetBuffer()
.getInt((long) targetVector.getValueCount() *
BaseVariableWidthVector.OFFSET_WIDTH);
+ // The delta vector's offset buffer need not start at zero (e.g. a vector
imported through
+ // the C data interface from a sliced array), so the amount of data to
append is the
+ // distance between its first and last offsets, not the last offset itself.
+ int deltaDataStart = deltaVector.getOffsetBuffer().getInt(0);
int deltaDataSize =
deltaVector
- .getOffsetBuffer()
- .getInt((long) deltaVector.getValueCount() *
BaseVariableWidthVector.OFFSET_WIDTH);
+ .getOffsetBuffer()
+ .getInt((long) deltaVector.getValueCount() *
BaseVariableWidthVector.OFFSET_WIDTH)
+ - deltaDataStart;
int newValueCapacity = targetDataSize + deltaDataSize;
// make sure there is enough capacity
@@ -149,7 +154,7 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
// append data buffer
MemoryUtil.copyMemory(
- deltaVector.getDataBuffer().memoryAddress(),
+ deltaVector.getDataBuffer().memoryAddress() + deltaDataStart,
targetVector.getDataBuffer().memoryAddress() + targetDataSize,
deltaDataSize);
@@ -160,7 +165,7 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
+ (targetVector.getValueCount() + 1) *
BaseVariableWidthVector.OFFSET_WIDTH,
deltaVector.getValueCount() * BaseVariableWidthVector.OFFSET_WIDTH);
- // increase each offset from the second buffer
+ // rebase each appended offset to the target's data, accounting for the
delta's start offset
for (int i = 0; i < deltaVector.getValueCount(); i++) {
int oldOffset =
targetVector
@@ -172,7 +177,7 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
.getOffsetBuffer()
.setInt(
(long) (targetVector.getValueCount() + 1 + i) *
BaseVariableWidthVector.OFFSET_WIDTH,
- oldOffset + targetDataSize);
+ oldOffset - deltaDataStart + targetDataSize);
}
((BaseVariableWidthVector) targetVector).setLastSet(newValueCount - 1);
targetVector.setValueCount(newValueCount);
@@ -196,11 +201,15 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
.getOffsetBuffer()
.getLong(
(long) targetVector.getValueCount() *
BaseLargeVariableWidthVector.OFFSET_WIDTH);
+ // see the corresponding comment in visit(BaseVariableWidthVector, Void):
the delta's
+ // offset buffer need not start at zero
+ long deltaDataStart = deltaVector.getOffsetBuffer().getLong(0);
long deltaDataSize =
deltaVector
- .getOffsetBuffer()
- .getLong(
- (long) deltaVector.getValueCount() *
BaseLargeVariableWidthVector.OFFSET_WIDTH);
+ .getOffsetBuffer()
+ .getLong(
+ (long) deltaVector.getValueCount() *
BaseLargeVariableWidthVector.OFFSET_WIDTH)
+ - deltaDataStart;
long newValueCapacity = targetDataSize + deltaDataSize;
// make sure there is enough capacity
@@ -221,7 +230,7 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
// append data buffer
MemoryUtil.copyMemory(
- deltaVector.getDataBuffer().memoryAddress(),
+ deltaVector.getDataBuffer().memoryAddress() + deltaDataStart,
targetVector.getDataBuffer().memoryAddress() + targetDataSize,
deltaDataSize);
@@ -232,7 +241,7 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
+ (targetVector.getValueCount() + 1) *
BaseLargeVariableWidthVector.OFFSET_WIDTH,
deltaVector.getValueCount() *
BaseLargeVariableWidthVector.OFFSET_WIDTH);
- // increase each offset from the second buffer
+ // rebase each appended offset to the target's data, accounting for the
delta's start offset
for (int i = 0; i < deltaVector.getValueCount(); i++) {
long oldOffset =
targetVector
@@ -245,7 +254,7 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
.setLong(
(long) (targetVector.getValueCount() + 1 + i)
* BaseLargeVariableWidthVector.OFFSET_WIDTH,
- oldOffset + targetDataSize);
+ oldOffset - deltaDataStart + targetDataSize);
}
((BaseLargeVariableWidthVector) targetVector).setLastSet(newValueCount -
1);
targetVector.setValueCount(newValueCount);
@@ -331,16 +340,20 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
targetVector
.getOffsetBuffer()
.getInt((long) targetVector.getValueCount() *
ListVector.OFFSET_WIDTH);
- int deltaListSize =
+ // see the corresponding comment in visit(BaseVariableWidthVector, Void):
the delta's
+ // offset buffer need not start at zero
+ int deltaListStart = deltaVector.getOffsetBuffer().getInt(0);
+ int deltaListEnd =
deltaVector
.getOffsetBuffer()
.getInt((long) deltaVector.getValueCount() *
ListVector.OFFSET_WIDTH);
+ int deltaListSize = deltaListEnd - deltaListStart;
ListVector targetListVector = (ListVector) targetVector;
// make sure the underlying vector has value count set
targetListVector.getDataVector().setValueCount(targetListSize);
- deltaVector.getDataVector().setValueCount(deltaListSize);
+ deltaVector.getDataVector().setValueCount(deltaListEnd);
// make sure there is enough capacity
while (targetVector.getValueCapacity() < newValueCount) {
@@ -372,13 +385,16 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
.getOffsetBuffer()
.setInt(
(long) (targetVector.getValueCount() + 1 + i) *
ListVector.OFFSET_WIDTH,
- oldOffset + targetListSize);
+ oldOffset - deltaListStart + targetListSize);
}
targetListVector.setLastSet(newValueCount - 1);
// append underlying vectors
- VectorAppender innerAppender = new
VectorAppender(targetListVector.getDataVector());
- deltaVector.getDataVector().accept(innerAppender, null);
+ appendDataVector(
+ targetListVector.getDataVector(),
+ deltaVector.getDataVector(),
+ deltaListStart,
+ deltaListSize);
targetVector.setValueCount(newValueCount);
return targetVector;
@@ -400,17 +416,21 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
targetVector
.getOffsetBuffer()
.getLong((long) targetVector.getValueCount() *
LargeListVector.OFFSET_WIDTH);
- long deltaListSize =
+ // see the corresponding comment in visit(BaseVariableWidthVector, Void):
the delta's
+ // offset buffer need not start at zero
+ long deltaListStart = deltaVector.getOffsetBuffer().getLong(0);
+ long deltaListEnd =
deltaVector
.getOffsetBuffer()
.getLong((long) deltaVector.getValueCount() *
LargeListVector.OFFSET_WIDTH);
+ long deltaListSize = deltaListEnd - deltaListStart;
- ListVector targetListVector = (ListVector) targetVector;
+ LargeListVector targetListVector = (LargeListVector) targetVector;
// make sure the underlying vector has value count set
// todo recheck these casts when int64 vectors are supported
targetListVector.getDataVector().setValueCount(checkedCastToInt(targetListSize));
- deltaVector.getDataVector().setValueCount(checkedCastToInt(deltaListSize));
+ deltaVector.getDataVector().setValueCount(checkedCastToInt(deltaListEnd));
// make sure there is enough capacity
while (targetVector.getValueCapacity() < newValueCount) {
@@ -427,10 +447,10 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
// append offset buffer
MemoryUtil.copyMemory(
- deltaVector.getOffsetBuffer().memoryAddress() +
ListVector.OFFSET_WIDTH,
+ deltaVector.getOffsetBuffer().memoryAddress() +
LargeListVector.OFFSET_WIDTH,
targetVector.getOffsetBuffer().memoryAddress()
+ (targetVector.getValueCount() + 1) *
LargeListVector.OFFSET_WIDTH,
- (long) deltaVector.getValueCount() * ListVector.OFFSET_WIDTH);
+ (long) deltaVector.getValueCount() * LargeListVector.OFFSET_WIDTH);
// increase each offset from the second buffer
for (int i = 0; i < deltaVector.getValueCount(); i++) {
@@ -443,18 +463,42 @@ public class VectorAppender implements
VectorVisitor<ValueVector, Void> {
.getOffsetBuffer()
.setLong(
(long) (targetVector.getValueCount() + 1 + i) *
LargeListVector.OFFSET_WIDTH,
- oldOffset + targetListSize);
+ oldOffset - deltaListStart + targetListSize);
}
targetListVector.setLastSet(newValueCount - 1);
// append underlying vectors
- VectorAppender innerAppender = new
VectorAppender(targetListVector.getDataVector());
- deltaVector.getDataVector().accept(innerAppender, null);
+ appendDataVector(
+ targetListVector.getDataVector(),
+ deltaVector.getDataVector(),
+ checkedCastToInt(deltaListStart),
+ checkedCastToInt(deltaListSize));
targetVector.setValueCount(newValueCount);
return targetVector;
}
+ /**
+ * Appends the range [start, start + length) of the delta vector's data
vector to the target
+ * vector's data vector. The range may not cover the whole delta data vector
when the delta's
+ * offset buffer does not start at zero.
+ */
+ private static void appendDataVector(
+ ValueVector targetDataVector, ValueVector deltaDataVector, int start,
int length) {
+ if (start == 0 && length == deltaDataVector.getValueCount()) {
+ VectorAppender innerAppender = new VectorAppender(targetDataVector);
+ deltaDataVector.accept(innerAppender, null);
+ return;
+ }
+ TransferPair transferPair =
+ deltaDataVector.getTransferPair(deltaDataVector.getField(),
deltaDataVector.getAllocator());
+ transferPair.splitAndTransfer(start, length);
+ try (ValueVector slicedDeltaDataVector = transferPair.getTo()) {
+ VectorAppender innerAppender = new VectorAppender(targetDataVector);
+ slicedDeltaDataVector.accept(innerAppender, null);
+ }
+ }
+
@Override
public ValueVector visit(FixedSizeListVector deltaVector, Void value) {
Preconditions.checkArgument(
diff --git
a/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
b/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
index df5521a1a..9a8143f51 100644
--- a/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
+++ b/vector/src/test/java/org/apache/arrow/vector/util/TestVectorAppender.java
@@ -26,10 +26,13 @@ import java.util.Collections;
import java.util.List;
import java.util.stream.IntStream;
import java.util.stream.Stream;
+import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.apache.arrow.memory.util.CommonUtil;
+import org.apache.arrow.vector.BaseLargeVariableWidthVector;
import org.apache.arrow.vector.BaseValueVector;
+import org.apache.arrow.vector.BaseVariableWidthVector;
import org.apache.arrow.vector.BaseVariableWidthViewVector;
import org.apache.arrow.vector.BigIntVector;
import org.apache.arrow.vector.BitVector;
@@ -53,6 +56,7 @@ import org.apache.arrow.vector.complex.UnionVector;
import org.apache.arrow.vector.holders.NullableBigIntHolder;
import org.apache.arrow.vector.holders.NullableFloat4Holder;
import org.apache.arrow.vector.holders.NullableIntHolder;
+import org.apache.arrow.vector.ipc.message.ArrowFieldNode;
import org.apache.arrow.vector.testing.ValueVectorDataPopulator;
import org.apache.arrow.vector.types.Types;
import org.apache.arrow.vector.types.pojo.ArrowType;
@@ -178,6 +182,82 @@ public class TestVectorAppender {
}
}
+ @Test
+ public void testAppendVariableWidthVectorWithNonZeroStartOffset() {
+ try (VarCharVector target = new VarCharVector("", allocator);
+ VarCharVector delta = new VarCharVector("", allocator)) {
+
+ target.allocateNew(64, 4);
+ ValueVectorDataPopulator.setVector(target, "a0", "a1");
+
+ // Build a delta vector whose offset buffer does not start at zero, as
produced e.g. by
+ // importing a sliced array through the C data interface. The values are
"BBBB" and
+ // "CCCC"; the data buffer additionally holds 4 bytes of unreferenced
prefix ("AAAA").
+ try (ArrowBuf validity = allocator.buffer(1);
+ ArrowBuf offsets = allocator.buffer(12);
+ ArrowBuf data = allocator.buffer(12)) {
+ validity.setByte(0, 0b11);
+ offsets.setInt(0, 4);
+ offsets.setInt(4, 8);
+ offsets.setInt(8, 12);
+ data.setBytes(0, "AAAABBBBCCCC".getBytes(StandardCharsets.UTF_8));
+ delta.loadFieldBuffers(new ArrowFieldNode(2, 0),
Arrays.asList(validity, offsets, data));
+ }
+
+ VectorAppender appender = new VectorAppender(target);
+ delta.accept(appender, null);
+
+ // the unreferenced prefix must not be appended
+ assertEquals(
+ 4 + 8,
+ target
+ .getOffsetBuffer()
+ .getInt((long) target.getValueCount() *
BaseVariableWidthVector.OFFSET_WIDTH));
+
+ try (VarCharVector expected = new VarCharVector("expected", allocator)) {
+ expected.allocateNew();
+ ValueVectorDataPopulator.setVector(expected, "a0", "a1", "BBBB",
"CCCC");
+ assertVectorsEqual(expected, target);
+ }
+ }
+ }
+
+ @Test
+ public void testAppendLargeVariableWidthVectorWithNonZeroStartOffset() {
+ try (LargeVarCharVector target = new LargeVarCharVector("", allocator);
+ LargeVarCharVector delta = new LargeVarCharVector("", allocator)) {
+
+ target.allocateNew(64, 4);
+ ValueVectorDataPopulator.setVector(target, "a0", "a1");
+
+ try (ArrowBuf validity = allocator.buffer(1);
+ ArrowBuf offsets = allocator.buffer(24);
+ ArrowBuf data = allocator.buffer(12)) {
+ validity.setByte(0, 0b11);
+ offsets.setLong(0, 4);
+ offsets.setLong(8, 8);
+ offsets.setLong(16, 12);
+ data.setBytes(0, "AAAABBBBCCCC".getBytes(StandardCharsets.UTF_8));
+ delta.loadFieldBuffers(new ArrowFieldNode(2, 0),
Arrays.asList(validity, offsets, data));
+ }
+
+ VectorAppender appender = new VectorAppender(target);
+ delta.accept(appender, null);
+
+ assertEquals(
+ 4 + 8,
+ target
+ .getOffsetBuffer()
+ .getLong((long) target.getValueCount() *
BaseLargeVariableWidthVector.OFFSET_WIDTH));
+
+ try (LargeVarCharVector expected = new LargeVarCharVector("expected",
allocator)) {
+ expected.allocateNew();
+ ValueVectorDataPopulator.setVector(expected, "a0", "a1", "BBBB",
"CCCC");
+ assertVectorsEqual(expected, target);
+ }
+ }
+ }
+
@Test
public void testAppendVariableWidthViewVector() {
final int length1 = 10;
@@ -431,6 +511,115 @@ public class TestVectorAppender {
}
}
+ @Test
+ public void testAppendListVectorWithNonZeroStartOffset() {
+ try (ListVector target = ListVector.empty("target", allocator);
+ ListVector delta = ListVector.empty("delta", allocator)) {
+
+ target.allocateNew();
+ ValueVectorDataPopulator.setVector(target, Arrays.asList(0, 1),
Arrays.asList(2, 3));
+
+ // Build a delta vector whose offset buffer does not start at zero, as
produced e.g. by
+ // importing a sliced array through the C data interface: lists [10, 11]
and [12, 13],
+ // with one unreferenced prefix element (9) in the data vector.
+ delta.addOrGetVector(FieldType.nullable(Types.MinorType.INT.getType()));
+ IntVector deltaDataVector = (IntVector) delta.getDataVector();
+ deltaDataVector.allocateNew(5);
+ for (int i = 0; i < 5; i++) {
+ deltaDataVector.set(i, 9 + i);
+ }
+ deltaDataVector.setValueCount(5);
+ try (ArrowBuf validity = allocator.buffer(1);
+ ArrowBuf offsets = allocator.buffer(12)) {
+ validity.setByte(0, 0b11);
+ offsets.setInt(0, 1);
+ offsets.setInt(4, 3);
+ offsets.setInt(8, 5);
+ delta.loadFieldBuffers(new ArrowFieldNode(2, 0),
Arrays.asList(validity, offsets));
+ }
+ assertEquals(Arrays.asList(10, 11), delta.getObject(0));
+
+ VectorAppender appender = new VectorAppender(target);
+ delta.accept(appender, null);
+
+ assertEquals(4, target.getValueCount());
+ // the unreferenced prefix element must not be appended
+ assertEquals(
+ 4 + 4,
+ target.getOffsetBuffer().getInt((long) target.getValueCount() *
ListVector.OFFSET_WIDTH));
+ assertEquals(Arrays.asList(0, 1), target.getObject(0));
+ assertEquals(Arrays.asList(2, 3), target.getObject(1));
+ assertEquals(Arrays.asList(10, 11), target.getObject(2));
+ assertEquals(Arrays.asList(12, 13), target.getObject(3));
+ }
+ }
+
+ @Test
+ public void testAppendLargeListVector() {
+ try (LargeListVector target = LargeListVector.empty("target", allocator);
+ LargeListVector delta = LargeListVector.empty("delta", allocator)) {
+
+ target.allocateNew();
+ ValueVectorDataPopulator.setVector(target, Arrays.asList(0, 1), null,
Arrays.asList(4, 5));
+
+ delta.allocateNew();
+ ValueVectorDataPopulator.setVector(delta, Arrays.asList(10, 11, 12),
Arrays.asList(13, 14));
+
+ VectorAppender appender = new VectorAppender(target);
+ delta.accept(appender, null);
+
+ assertEquals(5, target.getValueCount());
+ assertEquals(Arrays.asList(0, 1), target.getObject(0));
+ assertTrue(target.isNull(1));
+ assertEquals(Arrays.asList(4, 5), target.getObject(2));
+ assertEquals(Arrays.asList(10, 11, 12), target.getObject(3));
+ assertEquals(Arrays.asList(13, 14), target.getObject(4));
+ }
+ }
+
+ @Test
+ public void testAppendLargeListVectorWithNonZeroStartOffset() {
+ try (LargeListVector target = LargeListVector.empty("target", allocator);
+ LargeListVector delta = LargeListVector.empty("delta", allocator)) {
+
+ target.allocateNew();
+ ValueVectorDataPopulator.setVector(target, Arrays.asList(0, 1),
Arrays.asList(2, 3));
+
+ // same as testAppendListVectorWithNonZeroStartOffset, with 8-byte
offsets
+ delta.addOrGetVector(FieldType.nullable(Types.MinorType.INT.getType()));
+ IntVector deltaDataVector = (IntVector) delta.getDataVector();
+ deltaDataVector.allocateNew(5);
+ for (int i = 0; i < 5; i++) {
+ deltaDataVector.set(i, 9 + i);
+ }
+ deltaDataVector.setValueCount(5);
+ try (ArrowBuf validity = allocator.buffer(1);
+ ArrowBuf offsets = allocator.buffer(24)) {
+ validity.setByte(0, 0b11);
+ offsets.setLong(0, 1);
+ offsets.setLong(8, 3);
+ offsets.setLong(16, 5);
+ delta.loadFieldBuffers(new ArrowFieldNode(2, 0),
Arrays.asList(validity, offsets));
+ }
+ assertEquals(Arrays.asList(10, 11), delta.getObject(0));
+
+ VectorAppender appender = new VectorAppender(target);
+ delta.accept(appender, null);
+
+ assertEquals(4, target.getValueCount());
+ // the unreferenced prefix element must not be appended
+ assertEquals(
+ 4 + 4,
+ target
+ .getOffsetBuffer()
+ .getLong((long) target.getValueCount() *
LargeListVector.OFFSET_WIDTH));
+ assertEquals(Arrays.asList(0, 1), target.getObject(0));
+ assertEquals(Arrays.asList(2, 3), target.getObject(1));
+ assertEquals(Arrays.asList(10, 11), target.getObject(2));
+ assertEquals(Arrays.asList(12, 13), target.getObject(3));
+ }
+ }
+
@Test
public void testAppendEmptyListVector() {
try (ListVector target = ListVector.empty("target", allocator);
