[GitHub] [arrow] rtadepalli commented on a diff in pull request #34424: GH-15187: [Java] Made `reader` initialization lazy and added new `getTransferPair()` function that takes in a `Field` type

["rtadepalli (via GitHub)" <gi...@apache.org>]

rtadepalli commented on code in PR #34424:
URL: https://github.com/apache/arrow/pull/34424#discussion_r1217272625


##########
java/vector/src/main/java/org/apache/arrow/vector/ExtensionTypeVector.java:
##########
@@ -130,6 +130,12 @@ public TransferPair makeTransferPair(ValueVector target) {
     return underlyingVector.makeTransferPair(target);
   }
 
+  @Override
+  protected Class<? extends FieldReader> getReaderImplClass() {
+    throw new UnsupportedOperationException("Readers for extension types depend on the underlying vector, " +

Review Comment:
   Throwing an exception here because `getReader` is delegating to the underlying vector, which to my understanding would always be a subclass of `BaseValueVector`. This means that there'd be a concrete implementation of `getReaderImplClass` that'd be available, so delegating to the `getReader` method of the underlying vector is fine and there's no need of an implementation here.
   
   Please let me know if this is not the case.



##########
java/vector/src/main/java/org/apache/arrow/vector/BigIntVector.java:
##########
@@ -71,7 +73,11 @@ public BigIntVector(String name, FieldType fieldType, BufferAllocator allocator)
    */
   public BigIntVector(Field field, BufferAllocator allocator) {
     super(field, allocator, TYPE_WIDTH);
-    reader = new BigIntReaderImpl(BigIntVector.this);
+    reader = () -> {

Review Comment:
   @prashanthbdremio I think adding a new constructor would not be backwards compatible, unless we just default the boolean to `true` in the `BigIntVector(field, allocator);` constructor (and constructors in other classes). If we do this, I guess we're not really changing much, since the reader will be eagerly created anyways. If you think that is fine, I can make the change.
   
   @lwhite1 I have made some changes to the way the reader is being initialized -- please take another look. Thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org