lidavidm commented on code in PR #766:
URL: https://github.com/apache/arrow-java/pull/766#discussion_r2107950825


##########
c/src/main/java/org/apache/arrow/c/Data.java:
##########
@@ -383,29 +521,52 @@ public static VectorSchemaRoot importVectorSchemaRoot(
    * @param array C data interface struct holding the record batch data 
(optional)
    * @param schema C data interface struct holding the record batch schema
    * @param provider Dictionary provider to load dictionary vectors to 
(optional)
+   * @param closeImportedStructs if true, the ArrowArray struct will be closed 
when this method completes successfully and
+   *                             the ArrowSchema struct will be always be 
closed.
    * @return Imported vector schema root
    */
   public static VectorSchemaRoot importVectorSchemaRoot(
-      BufferAllocator allocator,
-      ArrowArray array,
-      ArrowSchema schema,
-      CDataDictionaryProvider provider) {
+            BufferAllocator allocator,
+            ArrowArray array,
+            ArrowSchema schema,
+            CDataDictionaryProvider provider,
+            boolean closeImportedStructs) {
     VectorSchemaRoot vsr =
-        VectorSchemaRoot.create(importSchema(allocator, schema, provider), 
allocator);
+        VectorSchemaRoot.create(importSchema(allocator, schema, provider, 
closeImportedStructs), allocator);
     if (array != null) {
-      importIntoVectorSchemaRoot(allocator, array, vsr, provider);
+      importIntoVectorSchemaRoot(allocator, array, vsr, provider, 
closeImportedStructs);
     }
     return vsr;
   }
 
   /**
-   * Import an ArrowArrayStream as an {@link ArrowReader}.
+   * Equivalent to calling {@link ##importArrayStream(BufferAllocator, 
ArrowArrayStream, boolean) importArrayStream(allocator, stream, true)}.
    *
    * @param allocator Buffer allocator for allocating the output data.
    * @param stream C stream interface struct to import.
    * @return Imported reader
+   * @see #importArrayStream(BufferAllocator, ArrowArrayStream, boolean)
    */
   public static ArrowReader importArrayStream(BufferAllocator allocator, 
ArrowArrayStream stream) {
-    return new ArrowArrayStreamReader(allocator, stream);
+    return importArrayStream(allocator, stream, true);
+  }
+
+  /**
+   * Import an ArrowArrayStream as an {@link ArrowReader}.
+   *
+   * <p>On successful completion, the ArrowArrayStream struct will have been 
moved (as per the C data interface specification)
+   *  to a private object held alive by the resulting ArrowReader.
+   *
+   * @param allocator Buffer allocator for allocating the output data.
+   * @param stream C stream interface struct to import.
+   * @param closeImportedStructs if true, the ArrowArrayStream struct will be 
closed when this method completes successfully
+   * @return Imported reader
+   */
+  public static ArrowReader importArrayStream(BufferAllocator allocator, 
ArrowArrayStream stream, boolean closeImportedStructs) {
+    ArrowArrayStreamReader reader = new ArrowArrayStreamReader(allocator, 
stream);
+      if (closeImportedStructs) {
+          stream.close();
+      }
+      return reader;

Review Comment:
   (1) the indentation seems off?
   (2) should we put this in a try-with-resources to at least fix the edge case 
noted (failure to free in case of exception)?



-- 
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-unsubscr...@arrow.apache.org

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

Reply via email to