diff --git a/arrow-variant/src/main/java/org/apache/arrow/variant/impl/NullableVariantHolderReaderImpl.java b/arrow-variant/src/main/java/org/apache/arrow/variant/impl/NullableVariantHolderReaderImpl.java index 1645529c0c..c9c392deb2 100644 --- a/arrow-variant/src/main/java/org/apache/arrow/variant/impl/NullableVariantHolderReaderImpl.java +++ b/arrow-variant/src/main/java/org/apache/arrow/variant/impl/NullableVariantHolderReaderImpl.java @@ -19,6 +19,7 @@ import org.apache.arrow.variant.holders.NullableVariantHolder; import org.apache.arrow.vector.complex.impl.AbstractFieldReader; import org.apache.arrow.vector.types.Types; +import org.apache.arrow.vector.types.pojo.ArrowType; public class NullableVariantHolderReaderImpl extends AbstractFieldReader { private final NullableVariantHolder holder; @@ -47,6 +48,11 @@ public Types.MinorType getMinorType() { return Types.MinorType.EXTENSIONTYPE; } + @Override + public ArrowType getExtensionType() { + return holder.type(); + } + @Override public boolean isSet() { return holder.isSet == 1; diff --git a/arrow-variant/src/test/java/org/apache/arrow/variant/extension/TestVariantVector.java b/arrow-variant/src/test/java/org/apache/arrow/variant/extension/TestVariantVector.java index 1c172e304f..c06a75ae25 100644 --- a/arrow-variant/src/test/java/org/apache/arrow/variant/extension/TestVariantVector.java +++ b/arrow-variant/src/test/java/org/apache/arrow/variant/extension/TestVariantVector.java @@ -30,6 +30,7 @@ import org.apache.arrow.variant.Variant; import org.apache.arrow.variant.holders.NullableVariantHolder; import org.apache.arrow.variant.holders.VariantHolder; +import org.apache.arrow.variant.impl.NullableVariantHolderReaderImpl; import org.apache.arrow.variant.impl.VariantReaderImpl; import org.apache.arrow.variant.impl.VariantWriterImpl; import org.apache.arrow.vector.holders.ExtensionHolder; @@ -841,4 +842,16 @@ void testClearAndReuse() { assertEquals(0, vector.getValueCount()); } } + + /** + * Holder readers must expose their extension {@link ArrowType} via {@code getExtensionType()} so + * callers (notably {@code ComplexCopier}) can route values through union/promotable writers + * without depending on {@code getField()}, which is undefined for holder-backed readers. + */ + @Test + void testNullableVariantHolderReaderImplGetExtensionType() { + NullableVariantHolder holder = new NullableVariantHolder(); + NullableVariantHolderReaderImpl reader = new NullableVariantHolderReaderImpl(holder); + assertEquals(VariantType.INSTANCE, reader.getExtensionType()); + } } diff --git a/vector/src/main/codegen/templates/ComplexCopier.java b/vector/src/main/codegen/templates/ComplexCopier.java index 6655f6c2a7..f4711339aa 100644 --- a/vector/src/main/codegen/templates/ComplexCopier.java +++ b/vector/src/main/codegen/templates/ComplexCopier.java @@ -111,7 +111,9 @@ public static void copy(FieldReader reader, FieldWriter writer) { if (reader.isSet()) { Object value = reader.readObject(); if (value != null) { - writer.writeExtension(value, reader.getField().getType()); + // Use getExtensionType() rather than getField().getType(): holder-backed + // readers carry their type on the ExtensionHolder and have no Field. + writer.writeExtension(value, reader.getExtensionType()); } } else { writer.writeNull(); @@ -170,7 +172,7 @@ private static FieldWriter getStructWriterForReader(FieldReader reader, StructWr case LISTVIEW: return (FieldWriter) writer.listView(name); case EXTENSIONTYPE: - ExtensionWriter extensionWriter = writer.extension(name, reader.getField().getType()); + ExtensionWriter extensionWriter = writer.extension(name, reader.getExtensionType()); return (FieldWriter) extensionWriter; default: throw new UnsupportedOperationException(reader.getMinorType().toString()); @@ -197,7 +199,7 @@ private static FieldWriter getListWriterForReader(FieldReader reader, ListWriter case LISTVIEW: return (FieldWriter) writer.listView(); case EXTENSIONTYPE: - ExtensionWriter extensionWriter = writer.extension(reader.getField().getType()); + ExtensionWriter extensionWriter = writer.extension(reader.getExtensionType()); return (FieldWriter) extensionWriter; default: throw new UnsupportedOperationException(reader.getMinorType().toString()); @@ -225,7 +227,7 @@ private static FieldWriter getMapWriterForReader(FieldReader reader, MapWriter w case MAP: return (FieldWriter) writer.map(false); case EXTENSIONTYPE: - ExtensionWriter extensionWriter = writer.extension(reader.getField().getType()); + ExtensionWriter extensionWriter = writer.extension(reader.getExtensionType()); return (FieldWriter) extensionWriter; default: throw new UnsupportedOperationException(reader.getMinorType().toString()); diff --git a/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableUuidHolderReaderImpl.java b/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableUuidHolderReaderImpl.java index 7a5312f6ed..36bf8f7539 100644 --- a/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableUuidHolderReaderImpl.java +++ b/vector/src/main/java/org/apache/arrow/vector/complex/impl/NullableUuidHolderReaderImpl.java @@ -20,6 +20,7 @@ import org.apache.arrow.vector.holders.NullableUuidHolder; import org.apache.arrow.vector.holders.UuidHolder; import org.apache.arrow.vector.types.Types; +import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.util.UuidUtility; /** @@ -72,6 +73,11 @@ public Types.MinorType getMinorType() { return Types.MinorType.EXTENSIONTYPE; } + @Override + public ArrowType getExtensionType() { + return holder.type(); + } + @Override public boolean isSet() { return holder.isSet == 1; diff --git a/vector/src/main/java/org/apache/arrow/vector/complex/reader/ExtensionReader.java b/vector/src/main/java/org/apache/arrow/vector/complex/reader/ExtensionReader.java index 1ba7b27156..0d6964dad3 100644 --- a/vector/src/main/java/org/apache/arrow/vector/complex/reader/ExtensionReader.java +++ b/vector/src/main/java/org/apache/arrow/vector/complex/reader/ExtensionReader.java @@ -17,6 +17,7 @@ package org.apache.arrow.vector.complex.reader; import org.apache.arrow.vector.holders.ExtensionHolder; +import org.apache.arrow.vector.types.pojo.ArrowType; /** Interface for reading extension types. Extends the functionality of {@link BaseReader}. */ public interface ExtensionReader extends BaseReader { @@ -41,4 +42,21 @@ public interface ExtensionReader extends BaseReader { * @return true if the value is set, false otherwise */ boolean isSet(); + + /** + * Returns the {@link ArrowType} of the extension data this reader exposes. + * + *

The default derives the type from {@link #getField()}, which works for vector-backed + * readers. Holder-backed readers (which have no notion of a {@code Field}) must override this to + * return the type carried by their {@link ExtensionHolder} directly. + * + *

Callers that need the extension {@link ArrowType} (e.g. to route a value through a union or + * promotable writer) should prefer this over {@code getField().getType()} so the call is + * well-defined regardless of how the reader is backed. + * + * @return the extension {@link ArrowType} + */ + default ArrowType getExtensionType() { + return getField().getType(); + } } diff --git a/vector/src/test/java/org/apache/arrow/vector/TestUuidVector.java b/vector/src/test/java/org/apache/arrow/vector/TestUuidVector.java index b5dd12d89c..50a84438ea 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestUuidVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestUuidVector.java @@ -723,4 +723,16 @@ void testNullableUuidHolderReaderImplWithNonZeroStart() throws Exception { assertEquals(uuid2, UuidUtility.uuidFromArrowBuf(targetHolder.buffer, targetHolder.start)); } } + + /** + * Holder readers must expose their extension {@link ArrowType} via {@code getExtensionType()} so + * callers (notably {@code ComplexCopier}) can route values through union/promotable writers + * without depending on {@code getField()}, which is undefined for holder-backed readers. + */ + @Test + void testNullableUuidHolderReaderImplGetExtensionType() { + NullableUuidHolder holder = new NullableUuidHolder(); + NullableUuidHolderReaderImpl reader = new NullableUuidHolderReaderImpl(holder); + assertEquals(UuidType.INSTANCE, reader.getExtensionType()); + } } diff --git a/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestComplexCopier.java b/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestComplexCopier.java index b2a8cf9ba4..e108894e49 100644 --- a/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestComplexCopier.java +++ b/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestComplexCopier.java @@ -16,6 +16,7 @@ */ package org.apache.arrow.vector.complex.impl; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -24,6 +25,7 @@ import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; import org.apache.arrow.vector.DecimalVector; +import org.apache.arrow.vector.UuidVector; import org.apache.arrow.vector.compare.VectorEqualsVisitor; import org.apache.arrow.vector.complex.FixedSizeListVector; import org.apache.arrow.vector.complex.ListVector; @@ -36,6 +38,7 @@ import org.apache.arrow.vector.complex.writer.FieldWriter; import org.apache.arrow.vector.extension.UuidType; import org.apache.arrow.vector.holders.DecimalHolder; +import org.apache.arrow.vector.holders.NullableUuidHolder; import org.apache.arrow.vector.types.Types; import org.apache.arrow.vector.types.pojo.ArrowType; import org.apache.arrow.vector.types.pojo.FieldType; @@ -954,4 +957,53 @@ public void testCopyStructVectorWithExtensionType() { assertTrue(VectorEqualsVisitor.vectorEquals(from, to)); } } + + /** + * {@link ComplexCopier#copy} must work when the source reader is a holder-backed extension reader + * (here {@link NullableUuidHolderReaderImpl}). Holder readers do not implement {@link + * FieldReader#getField()}, so the copier obtains the extension {@link ArrowType} via {@code + * getExtensionType()} instead. + */ + @Test + public void testCopyExtensionTypeFromNullableUuidHolderReader() { + try (UuidVector source = new UuidVector("v", allocator); + UuidVector target = new UuidVector("v", allocator)) { + UUID uuid = UUID.randomUUID(); + source.setSafe(0, uuid); + source.setValueCount(1); + + NullableUuidHolder holder = new NullableUuidHolder(); + source.get(0, holder); + + NullableUuidHolderReaderImpl reader = new NullableUuidHolderReaderImpl(holder); + UuidWriterImpl writer = new UuidWriterImpl(target); + writer.setPosition(0); + + ComplexCopier.copy(reader, writer); + target.setValueCount(1); + + assertEquals(uuid, target.getObject(0)); + } + } + + /** + * {@link ComplexCopier#copy} on a null-valued holder reader must take the not-set branch and + * write a null without invoking {@code getField()} or {@code readObject()}. + */ + @Test + public void testCopyExtensionTypeFromNullValuedNullableUuidHolderReader() { + try (UuidVector target = new UuidVector("v", allocator)) { + NullableUuidHolder holder = new NullableUuidHolder(); + holder.isSet = 0; + + NullableUuidHolderReaderImpl reader = new NullableUuidHolderReaderImpl(holder); + UuidWriterImpl writer = new UuidWriterImpl(target); + writer.setPosition(0); + + ComplexCopier.copy(reader, writer); + target.setValueCount(1); + + assertTrue(target.isNull(0)); + } + } }