Hello, @lidavidm! Could you take a look at this PR? Also, I don't have permissions to change the label

@jhrotko I will take a look on this one as soon as the CI is green (it should be good very soon).

This PR changes how we handle extension type writers in Arrow Java. Instead of using factories that get passed around everywhere, we now let the ArrowType.ExtensionType itself provide the writer implementation. This makes the API simpler and easier to work with, especially if you're implementing custom extension types outside arrow-java.

Problem

In Arrow's type system, each MinorType (INT, FLOAT, VARCHAR, etc.) has its own writer implementation. Extension types are trickier though, because they all share the same MinorType.EXTENSIONTYPE, but each extension type (UUID, Opaque, custom types) needs its own writer implementation. We needed some way to figure out which writer to use for a given extension type.

The previous implementation (commits 34060eb4, 7a7e4edd, 7fe36d70, 8663ffc6) used an ExtensionTypeWriterFactory pattern:

// Usage in ComplexCopier
writer.addExtensionTypeWriterFactory(extensionTypeWriterFactory);
writer.writeExtension(value);

In this pattern, each extension type had a separate factory class (like UuidWriterFactory) that was passed around as a parameter for copy methods. The custom extension writers stored these factories and used them to create the appropriate writer.
The TransferPair interface implementations also needed to carry factories, which polluted other ValueVector classes such as IntVectors and so on.

Why the factory pattern wasn't working well

For developers implementing extension types outside of arrow-java, the situation was even more painful. You had to create and manage two separate classes: one for the type itself (MyCustomType extends ExtensionType) and another for the factory (MyCustomWriterFactory implements ExtensionTypeWriterFactory).

The factory pattern had several issues that made it difficult to scale at this point. Specially if you wanted to use Extension Arrow-java types mixed with out of arrow-java extension types which is something that might happen more often in the future.

The API also got cluttered with factory parameters. Methods like ComplexCopier.copy(reader, writer, extensionTypeWriterFactory), writer.addExtensionTypeWriterFactory(factory), and TransferPair.makeTransferPair(target, factory) all needed these extra parameters. This made the API harder to use and understand.

Finally, the factory pattern created tight coupling between the type definition, the writer implementation, the factory that connects them, and all the code that needs to pass factories around. This made it harder to change any one piece without affecting the others.

The new approach: Let types provide their own writers

I added one abstract method to ArrowType.ExtensionType:

public abstract class ExtensionType extends ArrowType {
    // NEW METHOD
    public abstract FieldWriter getNewFieldWriter(ValueVector vector);

   // Other methods...
}

public class UuidType extends ExtensionType {
    @Override
    public FieldWriter getNewFieldWriter(ValueVector vector) {
        return new UuidWriterImpl((UuidVector) vector);
    }
    
    // Other methods...
}

The new approach is simpler because you only need one class per extension type now, not two. The type knows how to create its own writer. This also means the API is cleaner since there are no more factory parameters cluttering everything. For example, ComplexCopier.copy(reader, writer) and writer.writeExtension(value, type) are much more straightforward, and the type provides the writer internally through extensionType.getNewFieldWriter(vector).

This approach is also consistent with how MinorType already works. The existing pattern for MinorType has each enum constant override getNewFieldWriter() to return its specific writer implementation. Extension types now follow the same pattern:

// MinorType enum (existing pattern)
public enum MinorType {
    INT(new Int(...)) {
        @Override
        public FieldWriter getNewFieldWriter(ValueVector vector) {
            return new IntWriterImpl((IntVector) vector);
        }
    },
    // ...
}

// ExtensionType (new pattern - same idea)
public class UuidType extends ExtensionType {
    @Override
    public FieldWriter getNewFieldWriter(ValueVector vector) {
        return new UuidWriterImpl((UuidVector) vector);
    }
}

Finally, there's less coupling overall. Writers don't need to store or manage factories anymore, TransferPair implementations are simpler, and the type information just flows naturally through the ArrowType object.

ComplexCopier got simpler

// OLD: Required factory parameter
case EXTENSIONTYPE:
   if (extensionTypeWriterFactory == null) {
      throw new IllegalArgumentException("Must provide ExtensionTypeWriterFactory");
    }
    if (reader.isSet()) {
      Object value = reader.readObject();
       if (value != null) {
         writer.addExtensionTypeWriterFactory(extensionTypeWriterFactory);
         writer.writeExtension(value);
       }
     }
   ...

// NEW: Type provides the writer
case EXTENSIONTYPE:
    if (reader.isSet()) {
        Object value = reader.readObject();
        if (value != null) {
            writer.writeExtension(value, reader.getField().getType());
        }
    }
...

@lidavidm @xxlaykxx could you also take a look?

@jarohen does XTDB use extension types?

@jarohen does XTDB use extension types?

@lidavidm it does, but we have our own mechanisms for that outside of arrow-java I'm afraid. We're mostly using arrow-java for the IPC and memory management these days - we needed too many bespoke access patterns of the vectors themselves (particularly DUV) and didn't feel it reasonable to expect you folks to bend over backwards just for us 😄

That said, XT's all open source, feel free to pinch what you like, and I'm more'n happy to talk more about it (maybe a different thread though), if there's anything we can contribute back 🙂

Thanks for the confirmation! Just wanted to evaluate how this might affect you if we went ahead, sounds like it wouldn't be a problem

@lidavidm I see that the CI failures are unrelated to the changes and other PRs are having the same issues

@lidavidm yes, I would like to include in the 19.0.0 Arrow Java release (as soon as CI will be green 😄 ).

Ok, I assume we'll wait for CI to be fixed, then we can rebase this.

CI should be OK now. Thanks @jhrotko for the rebase, I just triggered a build.

@lidavidm @jbonofre there was a merge conflict, need to re-run the pipeline

@lidavidm @jbonofre can we merge this PR?

@jbonofre @lidavidm @laurentgo @xxlaykxx thank you so much for the reviews and support!