From afaf221755f274f3b7af9e9381228812efcf5431 Mon Sep 17 00:00:00 2001 From: bodduv Date: Thu, 27 Mar 2025 12:56:38 +0100 Subject: [PATCH 1/2] Preserve nullability information while transfering DecimalVector and Decimal256Vector --- .../apache/arrow/vector/Decimal256Vector.java | 4 +-- .../apache/arrow/vector/DecimalVector.java | 3 +- .../arrow/vector/TestDecimal256Vector.java | 28 +++++++++++++++++++ .../arrow/vector/TestDecimalVector.java | 26 +++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java index 90f67798dd..95ef6abcf5 100644 --- a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java +++ b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java @@ -566,9 +566,7 @@ private class TransferImpl implements TransferPair { Decimal256Vector to; public TransferImpl(String ref, BufferAllocator allocator) { - to = - new Decimal256Vector( - ref, allocator, Decimal256Vector.this.precision, Decimal256Vector.this.scale); + to = new Decimal256Vector(ref, Decimal256Vector.this.field.getFieldType(), allocator); } public TransferImpl(Field field, BufferAllocator allocator) { diff --git a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java index b4c55680b7..8ada607ed2 100644 --- a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java @@ -564,8 +564,7 @@ private class TransferImpl implements TransferPair { DecimalVector to; public TransferImpl(String ref, BufferAllocator allocator) { - to = - new DecimalVector(ref, allocator, DecimalVector.this.precision, DecimalVector.this.scale); + to = new DecimalVector(ref, DecimalVector.this.field.getFieldType(), allocator); } public TransferImpl(Field field, BufferAllocator allocator) { diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java index c155ab98fa..b995dc5d92 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java @@ -26,6 +26,7 @@ import org.apache.arrow.memory.ArrowBuf; import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.FieldType; import org.apache.arrow.vector.util.TransferPair; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -375,6 +376,33 @@ public void testGetTransferPairWithField() { assertSame(fromVector.getField(), toVector.getField()); } + @Test + public void testGetTransferPairWithoutField() { + final Decimal256Vector fromVector = new Decimal256Vector("decimal", allocator, 10, scale); + final TransferPair transferPair = + fromVector.getTransferPair(fromVector.getField().getName(), allocator); + final Decimal256Vector toVector = (Decimal256Vector) transferPair.getTo(); + // A new Field created inside a new vector should reuse the field type (should be the same in + // memory as the original Field's field type). + assertSame(fromVector.getField().getFieldType(), toVector.getField().getFieldType()); + } + + @Test + public void testGetTransferPairWithoutFieldNonNullable() { + final FieldType decimal256NonNullableType = + new FieldType( + false, new ArrowType.Decimal(10, scale, Decimal256Vector.TYPE_WIDTH * 8), null); + final Decimal256Vector fromVector = + new Decimal256Vector("decimal", decimal256NonNullableType, allocator); + final TransferPair transferPair = + fromVector.getTransferPair(fromVector.getField().getName(), allocator); + final Decimal256Vector toVector = (Decimal256Vector) transferPair.getTo(); + // A new Field created inside a new vector should reuse the field type (should be the same in + // memory as the original Field's field type). + assertSame(fromVector.getField().getFieldType(), toVector.getField().getFieldType()); + assertSame(decimal256NonNullableType, toVector.getField().getFieldType()); + } + private void verifyWritingArrowBufWithBigEndianBytes( Decimal256Vector decimalVector, ArrowBuf buf, BigDecimal[] expectedValues, int length) { decimalVector.allocateNew(); diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java index d5310bad0e..85c11e8f3d 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java @@ -26,6 +26,7 @@ import org.apache.arrow.memory.ArrowBuf; import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.FieldType; import org.apache.arrow.vector.util.TransferPair; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -371,6 +372,31 @@ public void testGetTransferPairWithField() { assertSame(fromVector.getField(), toVector.getField()); } + @Test + public void testGetTransferPairWithoutField() { + final DecimalVector fromVector = new DecimalVector("decimal", allocator, 10, scale); + final TransferPair transferPair = + fromVector.getTransferPair(fromVector.getField().getName(), allocator); + final DecimalVector toVector = (DecimalVector) transferPair.getTo(); + // A new Field created inside a new vector should reuse the field type (should be the same in + // memory as the original Field's field type). + assertSame(fromVector.getField().getFieldType(), toVector.getField().getFieldType()); + } + + @Test + public void testGetTransferPairWithoutFieldNonNullable() { + final FieldType decimalNonNullableType = + new FieldType(false, new ArrowType.Decimal(10, scale), null); + final DecimalVector fromVector = + new DecimalVector("decimal", decimalNonNullableType, allocator); + final TransferPair transferPair = + fromVector.getTransferPair(fromVector.getField().getName(), allocator); + final DecimalVector toVector = (DecimalVector) transferPair.getTo(); + // A new Field created inside a new vector should reuse the field type (should be the same in + // memory as the original Field's field type). + assertSame(fromVector.getField().getFieldType(), toVector.getField().getFieldType()); + } + private void verifyWritingArrowBufWithBigEndianBytes( DecimalVector decimalVector, ArrowBuf buf, BigDecimal[] expectedValues, int length) { decimalVector.allocateNew(); From 58f161583989b133d06ed220e2892a39d3e0c10a Mon Sep 17 00:00:00 2001 From: bodduv Date: Mon, 31 Mar 2025 16:56:34 +0200 Subject: [PATCH 2/2] Guard transfering DecimalVector and Decimal256Vector behind null checks on 'field' --- .../java/org/apache/arrow/vector/Decimal256Vector.java | 7 ++++++- .../main/java/org/apache/arrow/vector/DecimalVector.java | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java index 95ef6abcf5..f9d7e5cb9e 100644 --- a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java +++ b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java @@ -566,7 +566,12 @@ private class TransferImpl implements TransferPair { Decimal256Vector to; public TransferImpl(String ref, BufferAllocator allocator) { - to = new Decimal256Vector(ref, Decimal256Vector.this.field.getFieldType(), allocator); + to = + (Decimal256Vector.this.field != null + && Decimal256Vector.this.field.getFieldType() != null) + ? new Decimal256Vector(ref, Decimal256Vector.this.field.getFieldType(), allocator) + : new Decimal256Vector( + ref, allocator, Decimal256Vector.this.precision, Decimal256Vector.this.scale); } public TransferImpl(Field field, BufferAllocator allocator) { diff --git a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java index 8ada607ed2..9bf1812cc6 100644 --- a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java @@ -564,7 +564,11 @@ private class TransferImpl implements TransferPair { DecimalVector to; public TransferImpl(String ref, BufferAllocator allocator) { - to = new DecimalVector(ref, DecimalVector.this.field.getFieldType(), allocator); + to = + (DecimalVector.this.field != null && DecimalVector.this.field.getFieldType() != null) + ? new DecimalVector(ref, DecimalVector.this.field.getFieldType(), allocator) + : new DecimalVector( + ref, allocator, DecimalVector.this.precision, DecimalVector.this.scale); } public TransferImpl(Field field, BufferAllocator allocator) {