You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2023/06/13 12:21:07 UTC

[GitHub] [arrow] lidavidm 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

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


##########
java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java:
##########
@@ -143,6 +147,43 @@ long computeCombinedBufferSize(int valueCount, int typeWidth) {
     return allocator.getRoundingPolicy().getRoundedSize(bufferSize);
   }
 
+  /**
+   * Each vector has a different reader that implements the FieldReader interface. Overridden methods must make
+   * sure to return the correct type of the reader implementation to instantiate the reader properly.
+   *
+   * @return Returns the implementation class type of the vector's reader.
+   */
+  protected abstract Class<? extends FieldReader> getReaderImplClass();
+
+  /**
+   * Default implementation to create a reader for the vector. Depends on the individual vector
+   * class' implementation of `getReaderImpl()` to initialize the reader appropriately.
+   *
+   * @return Concrete instance of FieldReader by using lazy initialization.
+   */
+  public FieldReader getReader() {
+    FieldReader reader = fieldReader;
+
+    if (reader != null) {
+      return reader;
+    }
+    synchronized (this) {
+      if (fieldReader == null) {
+        try {
+          fieldReader =
+              (FieldReader) Class.forName(getReaderImplClass().getName()).getConstructor(getClass())

Review Comment:
   getReaderImplClass already returns a Class, why do we have to look it up again?



##########
java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java:
##########
@@ -143,6 +147,43 @@ long computeCombinedBufferSize(int valueCount, int typeWidth) {
     return allocator.getRoundingPolicy().getRoundedSize(bufferSize);
   }
 
+  /**
+   * Each vector has a different reader that implements the FieldReader interface. Overridden methods must make
+   * sure to return the correct type of the reader implementation to instantiate the reader properly.
+   *
+   * @return Returns the implementation class type of the vector's reader.
+   */
+  protected abstract Class<? extends FieldReader> getReaderImplClass();
+
+  /**
+   * Default implementation to create a reader for the vector. Depends on the individual vector
+   * class' implementation of `getReaderImpl()` to initialize the reader appropriately.

Review Comment:
   Should be `{@link #getReaderImplClass}`? JavaDoc does not use Markdown.



##########
java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java:
##########
@@ -143,6 +147,43 @@ long computeCombinedBufferSize(int valueCount, int typeWidth) {
     return allocator.getRoundingPolicy().getRoundedSize(bufferSize);
   }
 
+  /**
+   * Each vector has a different reader that implements the FieldReader interface. Overridden methods must make
+   * sure to return the correct type of the reader implementation to instantiate the reader properly.
+   *
+   * @return Returns the implementation class type of the vector's reader.
+   */
+  protected abstract Class<? extends FieldReader> getReaderImplClass();
+
+  /**
+   * Default implementation to create a reader for the vector. Depends on the individual vector
+   * class' implementation of `getReaderImpl()` to initialize the reader appropriately.
+   *
+   * @return Concrete instance of FieldReader by using lazy initialization.
+   */
+  public FieldReader getReader() {
+    FieldReader reader = fieldReader;
+
+    if (reader != null) {
+      return reader;
+    }
+    synchronized (this) {
+      if (fieldReader == null) {
+        try {
+          fieldReader =
+              (FieldReader) Class.forName(getReaderImplClass().getName()).getConstructor(getClass())

Review Comment:
   Also: why can't getReaderImplClass return a factory function instead of a class?



-- 
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