Skip to content

GH-891: Add ExtensionTypeWriterFactory to TransferPair #892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

GH-891: Add ExtensionTypeWriterFactory to TransferPair #892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7a7e4ed
GH-891: Add ExtensionTypeWriterFactory to TransferPair
jhrotko Oct 22, 2025
7fe36d7
Add makeTransferPair w/ extended factory
jhrotko Oct 28, 2025
8663ffc
Move ExtensionFactory to TransferPair
jhrotko Oct 29, 2025
a80b90c
Refactor copyvaluesafe w/ extension
jhrotko Oct 30, 2025
3af72e7
Clean up
jhrotko Oct 30, 2025
cedb5a7
remove writeValue from ComplexCopier
jhrotko Oct 30, 2025
f467600
Add javadoc
jhrotko Oct 30, 2025
ddfca39
Revert small change
jhrotko Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions vector/src/main/codegen/templates/AbstractFieldReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ public void copyAsField(String name, ${name}Writer writer) {

</#list></#list>

public void copyAsValue(StructWriter writer, ExtensionTypeWriterFactory writerFactory) {
Copy link
Contributor

@laurentgo laurentgo Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is okay to remove a public method because there has been no release yet?

Maybe we should discuss it on the mailing list as it seems we haven't found the right pattern yet

fail("CopyAsValue StructWriter");
}

public void read(ExtensionHolder holder) {
fail("Extension");
}
Expand Down Expand Up @@ -144,7 +140,13 @@ public int size() {
return -1;
}

@Override
public ExtensionTypeWriterFactory getExtensionTypeWriterFactory() {
throw new IllegalStateException("The current reader doesn't support reading extension type");
}

private void fail(String name) {
throw new IllegalArgumentException(String.format("You tried to read a [%s] type when you are using a field reader of type [%s].", name, this.getClass().getSimpleName()));
}

}
3 changes: 0 additions & 3 deletions vector/src/main/codegen/templates/BaseReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public interface RepeatedStructReader extends StructReader{
boolean next();
int size();
void copyAsValue(StructWriter writer);
void copyAsValue(StructWriter writer, ExtensionTypeWriterFactory writerFactory);
}

public interface ListReader extends BaseReader{
Expand All @@ -60,7 +59,6 @@ public interface RepeatedListReader extends ListReader{
boolean next();
int size();
void copyAsValue(ListWriter writer);
void copyAsValue(ListWriter writer, ExtensionTypeWriterFactory writerFactory);
}

public interface MapReader extends BaseReader{
Expand All @@ -71,7 +69,6 @@ public interface RepeatedMapReader extends MapReader{
boolean next();
int size();
void copyAsValue(MapWriter writer);
void copyAsValue(MapWriter writer, ExtensionTypeWriterFactory writerFactory);
}

public interface ScalarReader extends
Expand Down
23 changes: 7 additions & 16 deletions vector/src/main/codegen/templates/ComplexCopier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,8 @@ public class ComplexCopier {
* @param input field to read from
* @param output field to write to
*/
public static void copy(FieldReader input, FieldWriter output) {
writeValue(input, output, null);
}

public static void copy(FieldReader input, FieldWriter output, ExtensionTypeWriterFactory extensionTypeWriterFactory) {
writeValue(input, output, extensionTypeWriterFactory);
}
public static void copy(FieldReader reader, FieldWriter writer) {

private static void writeValue(FieldReader reader, FieldWriter writer, ExtensionTypeWriterFactory extensionTypeWriterFactory) {
final MinorType mt = reader.getMinorType();

switch (mt) {
Expand All @@ -65,7 +58,7 @@ private static void writeValue(FieldReader reader, FieldWriter writer, Extension
FieldReader childReader = reader.reader();
FieldWriter childWriter = getListWriterForReader(childReader, writer);
if (childReader.isSet()) {
writeValue(childReader, childWriter, extensionTypeWriterFactory);
copy(childReader, childWriter);
} else {
childWriter.writeNull();
}
Expand All @@ -83,8 +76,8 @@ private static void writeValue(FieldReader reader, FieldWriter writer, Extension
FieldReader structReader = reader.reader();
if (structReader.isSet()) {
writer.startEntry();
writeValue(mapReader.key(), getMapWriterForReader(mapReader.key(), writer.key()), extensionTypeWriterFactory);
writeValue(mapReader.value(), getMapWriterForReader(mapReader.value(), writer.value()), extensionTypeWriterFactory);
copy(mapReader.key(), getMapWriterForReader(mapReader.key(), writer.key()));
copy(mapReader.value(), getMapWriterForReader(mapReader.value(), writer.value()));
writer.endEntry();
} else {
writer.writeNull();
Expand All @@ -103,7 +96,7 @@ private static void writeValue(FieldReader reader, FieldWriter writer, Extension
if (childReader.getMinorType() != Types.MinorType.NULL) {
FieldWriter childWriter = getStructWriterForReader(childReader, writer, name);
if (childReader.isSet()) {
writeValue(childReader, childWriter, extensionTypeWriterFactory);
copy(childReader, childWriter);
} else {
childWriter.writeNull();
}
Expand All @@ -115,13 +108,11 @@ private static void writeValue(FieldReader reader, FieldWriter writer, Extension
}
break;
case EXTENSIONTYPE:
if (extensionTypeWriterFactory == null) {
throw new IllegalArgumentException("Must provide ExtensionTypeWriterFactory");
}
if (reader.isSet()) {
Object value = reader.readObject();
if (value != null) {
writer.addExtensionTypeWriterFactory(extensionTypeWriterFactory);
ExtensionTypeWriterFactory writerFactory = reader.getExtensionTypeWriterFactory();
Copy link
Author

@jhrotko jhrotko Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change: now the responsibility comes from the reader instead of passing down the factory.

writer.addExtensionTypeWriterFactory(writerFactory);
writer.writeExtension(value);
}
} else {
Expand Down
13 changes: 0 additions & 13 deletions vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.ReferenceManager;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.complex.reader.FieldReader;
import org.apache.arrow.vector.util.DataSizeRoundingUtil;
import org.apache.arrow.vector.util.TransferPair;
Expand Down Expand Up @@ -261,18 +260,6 @@ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
throw new UnsupportedOperationException();
}

@Override
public void copyFrom(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException();
}

@Override
public void copyFromSafe(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException();
}

/**
* Transfer the validity buffer from `validityBuffer` to the target vector's `validityBuffer`.
* Start at `startIndex` and copy `length` number of elements. If the starting index is 8 byte
Expand Down
13 changes: 0 additions & 13 deletions vector/src/main/java/org/apache/arrow/vector/NullVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.apache.arrow.memory.util.hash.ArrowBufHasher;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.vector.compare.VectorVisitor;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.complex.impl.NullReader;
import org.apache.arrow.vector.complex.reader.FieldReader;
import org.apache.arrow.vector.ipc.message.ArrowFieldNode;
Expand Down Expand Up @@ -330,18 +329,6 @@ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
throw new UnsupportedOperationException();
}

@Override
public void copyFrom(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException();
}

@Override
public void copyFromSafe(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException();
}

@Override
public String getName() {
return this.getField().getName();
Expand Down
25 changes: 0 additions & 25 deletions vector/src/main/java/org/apache/arrow/vector/ValueVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.arrow.memory.OutOfMemoryException;
import org.apache.arrow.memory.util.hash.ArrowBufHasher;
import org.apache.arrow.vector.compare.VectorVisitor;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.complex.reader.FieldReader;
import org.apache.arrow.vector.types.Types.MinorType;
import org.apache.arrow.vector.types.pojo.Field;
Expand Down Expand Up @@ -310,30 +309,6 @@ public interface ValueVector extends Closeable, Iterable<ValueVector> {
*/
void copyFromSafe(int fromIndex, int thisIndex, ValueVector from);

/**
* Copy a cell value from a particular index in source vector to a particular position in this
* vector.
*
* @param fromIndex position to copy from in source vector
* @param thisIndex position to copy to in this vector
* @param from source vector
* @param writerFactory the extension type writer factory to use for copying extension type values
*/
void copyFrom(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory);

/**
* Same as {@link #copyFrom(int, int, ValueVector)} except that it handles the case when the
* capacity of the vector needs to be expanded before copy.
*
* @param fromIndex position to copy from in source vector
* @param thisIndex position to copy to in this vector
* @param from source vector
* @param writerFactory the extension type writer factory to use for copying extension type values
*/
void copyFromSafe(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory);

/**
* Accept a generic {@link VectorVisitor} and return the result.
*
Expand Down
13 changes: 0 additions & 13 deletions vector/src/main/java/org/apache/arrow/vector/complex/AbstractContainerVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.apache.arrow.vector.DensityAwareVector;
import org.apache.arrow.vector.FieldVector;
import org.apache.arrow.vector.ValueVector;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.types.Types.MinorType;
import org.apache.arrow.vector.types.pojo.ArrowType;
import org.apache.arrow.vector.types.pojo.ArrowType.FixedSizeList;
Expand Down Expand Up @@ -152,18 +151,6 @@ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
throw new UnsupportedOperationException();
}

@Override
public void copyFrom(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException();
}

@Override
public void copyFromSafe(
int fromIndex, int thisIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException();
}

@Override
public String getName() {
return name;
Expand Down
33 changes: 1 addition & 32 deletions vector/src/main/java/org/apache/arrow/vector/complex/LargeListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import org.apache.arrow.vector.ZeroVector;
import org.apache.arrow.vector.compare.VectorVisitor;
import org.apache.arrow.vector.complex.impl.ComplexCopier;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.complex.impl.UnionLargeListReader;
import org.apache.arrow.vector.complex.impl.UnionLargeListWriter;
import org.apache.arrow.vector.complex.reader.FieldReader;
Expand Down Expand Up @@ -483,42 +482,12 @@ public void copyFromSafe(int inIndex, int outIndex, ValueVector from) {
*/
@Override
public void copyFrom(int inIndex, int outIndex, ValueVector from) {
copyFrom(inIndex, outIndex, from, null);
}

/**
* Copy a cell value from a particular index in source vector to a particular position in this
* vector.
*
* @param inIndex position to copy from in source vector
* @param outIndex position to copy to in this vector
* @param from source vector
* @param writerFactory the extension type writer factory to use for copying extension type values
*/
@Override
public void copyFrom(
int inIndex, int outIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
FieldReader in = from.getReader();
in.setPosition(inIndex);
UnionLargeListWriter out = getWriter();
out.setPosition(outIndex);
ComplexCopier.copy(in, out, writerFactory);
}

/**
* Same as {@link #copyFrom(int, int, ValueVector)} except that it handles the case when the
* capacity of the vector needs to be expanded before copy.
*
* @param inIndex position to copy from in source vector
* @param outIndex position to copy to in this vector
* @param from source vector
* @param writerFactory the extension type writer factory to use for copying extension type values
*/
@Override
public void copyFromSafe(
int inIndex, int outIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
copyFrom(inIndex, outIndex, from, writerFactory);
ComplexCopier.copy(in, out);
}

/**
Expand Down
15 changes: 0 additions & 15 deletions vector/src/main/java/org/apache/arrow/vector/complex/LargeListViewVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.apache.arrow.vector.ValueVector;
import org.apache.arrow.vector.ZeroVector;
import org.apache.arrow.vector.compare.VectorVisitor;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.complex.impl.UnionLargeListViewReader;
import org.apache.arrow.vector.complex.impl.UnionLargeListViewWriter;
import org.apache.arrow.vector.complex.impl.UnionListReader;
Expand Down Expand Up @@ -347,20 +346,6 @@ public void copyFrom(int inIndex, int outIndex, ValueVector from) {
"LargeListViewVector does not support copyFrom operation yet.");
}

@Override
public void copyFromSafe(
int inIndex, int outIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException(
"LargeListViewVector does not support copyFromSafe operation yet.");
}

@Override
public void copyFrom(
int inIndex, int outIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
throw new UnsupportedOperationException(
"LargeListViewVector does not support copyFrom operation yet.");
}

@Override
public FieldVector getDataVector() {
return vector;
Expand Down
33 changes: 1 addition & 32 deletions vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import org.apache.arrow.vector.ZeroVector;
import org.apache.arrow.vector.compare.VectorVisitor;
import org.apache.arrow.vector.complex.impl.ComplexCopier;
import org.apache.arrow.vector.complex.impl.ExtensionTypeWriterFactory;
import org.apache.arrow.vector.complex.impl.UnionListReader;
import org.apache.arrow.vector.complex.impl.UnionListWriter;
import org.apache.arrow.vector.complex.reader.FieldReader;
Expand Down Expand Up @@ -401,42 +400,12 @@ public void copyFromSafe(int inIndex, int outIndex, ValueVector from) {
*/
@Override
public void copyFrom(int inIndex, int outIndex, ValueVector from) {
copyFrom(inIndex, outIndex, from, null);
}

/**
* Same as {@link #copyFrom(int, int, ValueVector)} except that it handles the case when the
* capacity of the vector needs to be expanded before copy.
*
* @param inIndex position to copy from in source vector
* @param outIndex position to copy to in this vector
* @param from source vector
* @param writerFactory the extension type writer factory to use for copying extension type values
*/
@Override
public void copyFromSafe(
int inIndex, int outIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
copyFrom(inIndex, outIndex, from, writerFactory);
}

/**
* Copy a cell value from a particular index in source vector to a particular position in this
* vector.
*
* @param inIndex position to copy from in source vector
* @param outIndex position to copy to in this vector
* @param from source vector
* @param writerFactory the extension type writer factory to use for copying extension type values
*/
@Override
public void copyFrom(
int inIndex, int outIndex, ValueVector from, ExtensionTypeWriterFactory writerFactory) {
Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
FieldReader in = from.getReader();
in.setPosition(inIndex);
FieldWriter out = getWriter();
out.setPosition(outIndex);
ComplexCopier.copy(in, out, writerFactory);
ComplexCopier.copy(in, out);
}

/**
Expand Down
Loading