From d4a6932761e044f655598e01b0ae62db24dbad98 Mon Sep 17 00:00:00 2001 From: Vishal Boddu <9461605+bodduv@users.noreply.github.com> Date: Thu, 3 Apr 2025 17:25:44 +0200 Subject: [PATCH] GH-692: Preserve nullability information while transfering DecimalVector and Decimal256Vector (#693) ## What's Changed This PR proposes to use the "from" `ValueVector`'s field while transferring `DecimalVector` and `Decimal256Vector` to preserve nullability information. This is to have similar behavior with all the other primitive value vectors. Note that the `FieldType` of the value vector has nullability information. Closes #692 . (cherry picked from commit c29fcfc72bed84c1dc9926bc28294b5ff193937f) --- .../apache/arrow/vector/Decimal256Vector.java | 7 +++-- .../apache/arrow/vector/DecimalVector.java | 5 +++- .../arrow/vector/TestDecimal256Vector.java | 28 +++++++++++++++++++ .../arrow/vector/TestDecimalVector.java | 26 +++++++++++++++++ 4 files changed, 63 insertions(+), 3 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 42ad741c85..c57c3f6405 100644 --- a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java +++ b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java @@ -567,8 +567,11 @@ private class TransferImpl implements TransferPair { public TransferImpl(String ref, BufferAllocator allocator) { to = - new Decimal256Vector( - ref, allocator, Decimal256Vector.this.precision, Decimal256Vector.this.scale); + (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 b4c55680b7..9bf1812cc6 100644 --- a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java @@ -565,7 +565,10 @@ private class TransferImpl implements TransferPair { public TransferImpl(String ref, BufferAllocator allocator) { to = - new DecimalVector(ref, allocator, DecimalVector.this.precision, DecimalVector.this.scale); + (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) { 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();