Skip to content

Commit 32e4ae4

Browse files
lriggssgcowell
andauthored
GH-655: Failure in UnionReader.read after DecimalVector promotion to UnionVector (#656)
When a DecimalVector is promoted to a UnionVector via a PromotableWriter, the UnionVector will have the decimal vector in it's internal struct vector, but the decimalVector field will not be set. If UnionReader.read is then used to read from the UnionVector, it will fail when it tries to read one of the promoted decimal values, due to decimalVector being null, and the exact decimal type not being provided. This failure is unnecessary though as we have a pre-existing decimal vector, the caller just does not know the exact type - and it shouldn't be required to. The change here is to check for a pre-existing decimal vector in the internal struct when getDecimalVector() is called. If one exists, set the decimalVector field and return. Otherwise, if none exists, throw the exception. ## What's Changed Updated UnionVector to handle this case. Closes GH-655 --------- Co-authored-by: Scott Cowell <[email protected]>
1 parent d13f0e0 commit 32e4ae4

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

vector/src/main/codegen/templates/UnionVector.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.arrow.util.Preconditions;
2424
import org.apache.arrow.vector.BaseValueVector;
2525
import org.apache.arrow.vector.BitVectorHelper;
26+
import org.apache.arrow.vector.DecimalVector;
2627
import org.apache.arrow.vector.FieldVector;
2728
import org.apache.arrow.vector.ValueVector;
2829
import org.apache.arrow.vector.complex.AbstractStructVector;
@@ -279,7 +280,10 @@ public StructVector getStruct() {
279280
<#if minor.class?starts_with("Decimal") || is_timestamp_tz(minor.class) || minor.class == "Duration" || minor.class == "FixedSizeBinary">
280281
public ${name}Vector get${name}Vector() {
281282
if (${uncappedName}Vector == null) {
282-
throw new IllegalArgumentException("No ${name} present. Provide ArrowType argument to create a new vector");
283+
${uncappedName}Vector = internalStruct.getChild(fieldName(MinorType.${name?upper_case}), ${name}Vector.class);
284+
if (${uncappedName}Vector == null) {
285+
throw new IllegalArgumentException("No ${name} present. Provide ArrowType argument to create a new vector");
286+
}
283287
}
284288
return ${uncappedName}Vector;
285289
}

vector/src/test/java/org/apache/arrow/vector/complex/impl/TestPromotableWriter.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121
import static org.junit.jupiter.api.Assertions.assertNull;
2222
import static org.junit.jupiter.api.Assertions.assertThrows;
2323

24+
import java.math.BigDecimal;
2425
import java.nio.ByteBuffer;
2526
import java.nio.ByteOrder;
2627
import java.nio.charset.StandardCharsets;
2728
import java.util.Objects;
2829
import org.apache.arrow.memory.ArrowBuf;
2930
import org.apache.arrow.memory.BufferAllocator;
31+
import org.apache.arrow.vector.DecimalVector;
3032
import org.apache.arrow.vector.DirtyRootAllocator;
3133
import org.apache.arrow.vector.LargeVarBinaryVector;
3234
import org.apache.arrow.vector.LargeVarCharVector;
@@ -39,14 +41,18 @@
3941
import org.apache.arrow.vector.complex.writer.BaseWriter.StructWriter;
4042
import org.apache.arrow.vector.holders.DurationHolder;
4143
import org.apache.arrow.vector.holders.FixedSizeBinaryHolder;
44+
import org.apache.arrow.vector.holders.NullableDecimalHolder;
45+
import org.apache.arrow.vector.holders.NullableIntHolder;
4246
import org.apache.arrow.vector.holders.NullableTimeStampMilliTZHolder;
4347
import org.apache.arrow.vector.holders.TimeStampMilliTZHolder;
48+
import org.apache.arrow.vector.holders.UnionHolder;
4449
import org.apache.arrow.vector.types.TimeUnit;
4550
import org.apache.arrow.vector.types.Types;
4651
import org.apache.arrow.vector.types.pojo.ArrowType;
4752
import org.apache.arrow.vector.types.pojo.ArrowType.ArrowTypeID;
4853
import org.apache.arrow.vector.types.pojo.Field;
4954
import org.apache.arrow.vector.types.pojo.FieldType;
55+
import org.apache.arrow.vector.util.DecimalUtility;
5056
import org.apache.arrow.vector.util.Text;
5157
import org.junit.jupiter.api.AfterEach;
5258
import org.junit.jupiter.api.BeforeEach;
@@ -729,4 +735,45 @@ public void testPromoteLargeVarBinaryHelpersDirect() throws Exception {
729735
assertEquals("row4", new String(Objects.requireNonNull(uv.get(3)), StandardCharsets.UTF_8));
730736
}
731737
}
738+
739+
@Test
740+
public void testPromoteToUnionFromDecimal() throws Exception {
741+
try (final NonNullableStructVector container =
742+
NonNullableStructVector.empty(EMPTY_SCHEMA_PATH, allocator);
743+
final DecimalVector v =
744+
container.addOrGet(
745+
"dec", FieldType.nullable(new ArrowType.Decimal(38, 1, 128)), DecimalVector.class);
746+
final PromotableWriter writer = new PromotableWriter(v, container)) {
747+
748+
container.allocateNew();
749+
container.setValueCount(1);
750+
751+
writer.setPosition(0);
752+
writer.writeDecimal(new BigDecimal("0.1"));
753+
writer.setPosition(1);
754+
writer.writeInt(1);
755+
756+
container.setValueCount(3);
757+
758+
UnionVector unionVector = (UnionVector) container.getChild("dec");
759+
UnionHolder holder = new UnionHolder();
760+
761+
unionVector.get(0, holder);
762+
NullableDecimalHolder decimalHolder = new NullableDecimalHolder();
763+
holder.reader.read(decimalHolder);
764+
765+
assertEquals(1, decimalHolder.isSet);
766+
assertEquals(
767+
new BigDecimal("0.1"),
768+
DecimalUtility.getBigDecimalFromArrowBuf(
769+
decimalHolder.buffer, 0, decimalHolder.scale, 128));
770+
771+
unionVector.get(1, holder);
772+
NullableIntHolder intHolder = new NullableIntHolder();
773+
holder.reader.read(intHolder);
774+
775+
assertEquals(1, intHolder.isSet);
776+
assertEquals(1, intHolder.value);
777+
}
778+
}
732779
}

0 commit comments

Comments
 (0)