Skip to content

Commit

Permalink
GH-44344: [Java] fix VectorSchemaRoot.getTransferPair for NullVector (#…
Browse files Browse the repository at this point in the history
…44631)

### Rationale for this change

Do not throw [UnsupportedOperationException("Tried to get allocator from NullVector")](https://github.com/apache/arrow/blob/release-18.0.0-rc0/java/vector/src/main/java/org/apache/arrow/vector/NullVector.java#L160) from [VectorSchemaRoot.slice()](https://github.com/apache/arrow/blob/release-18.0.0-rc0/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java#L341) when slicing a VSR containing a NullVector or ZeroVector. Details in #44344

### Are these changes tested?

Added unit test that would trigger an UnsupportedOperationException on the legacy path.
* GitHub Issue: #44344

Authored-by: Maksim Yegorov <[email protected]>
Signed-off-by: David Li <[email protected]>
  • Loading branch information
myegorov authored Nov 6, 2024
1 parent f3b8d6b commit c3601a9
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,16 @@ public boolean allocateNewSafe() {
@Override
public void reAlloc() {}

/*
* IMPORTANT NOTE
* It's essential that NullVector (and ZeroVector) do not require BufferAllocator for any data storage.
* However, some methods of the parent interface may require passing in a BufferAllocator, even if null.
*
* @return null
*/
@Override
public BufferAllocator getAllocator() {
throw new UnsupportedOperationException("Tried to get allocator from NullVector");
return null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ public interface ValueVector extends Closeable, Iterable<ValueVector> {
*/
void reAlloc();

/**
* Get the allocator associated with the vector. CAVEAT: Some ValueVector subclasses (e.g.
* NullVector) do not require an allocator for data storage and may return null.
*
* @return Returns nullable allocator.
*/
BufferAllocator getAllocator();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,31 @@ public void testWithEmptyVector() {
toDUV.clear();
}

@Test
public void testWithNullVector() {
int valueCount = 123;
int startIndex = 10;
NullVector fromNullVector = new NullVector("nullVector");
fromNullVector.setValueCount(valueCount);
TransferPair transferPair = fromNullVector.getTransferPair(fromNullVector.getAllocator());
transferPair.splitAndTransfer(startIndex, valueCount - startIndex);
NullVector toNullVector = (NullVector) transferPair.getTo();

assertEquals(valueCount - startIndex, toNullVector.getValueCount());
// no allocations to clear for NullVector
}

@Test
public void testWithZeroVector() {
ZeroVector fromZeroVector = new ZeroVector("zeroVector");
TransferPair transferPair = fromZeroVector.getTransferPair(fromZeroVector.getAllocator());
transferPair.splitAndTransfer(0, 0);
ZeroVector toZeroVector = (ZeroVector) transferPair.getTo();

assertEquals(0, toZeroVector.getValueCount());
// no allocations to clear for ZeroVector
}

@Test /* VarCharVector */
public void test() throws Exception {
try (final VarCharVector varCharVector = new VarCharVector("myvector", allocator)) {
Expand Down

0 comments on commit c3601a9

Please sign in to comment.