You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/08/11 08:00:57 UTC

[GitHub] [arrow] liyafan82 opened a new pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

liyafan82 opened a new pull request #10912:
URL: https://github.com/apache/arrow/pull/10912


   See https://issues.apache.org/jira/browse/ARROW-13544
   
   According to the discussion in #10864 (comment), we want to split the task into multiple parts.
   
   This PR is for the changes related to the JDBC adapter


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



[GitHub] [arrow] emkornfield commented on pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #10912:
URL: https://github.com/apache/arrow/pull/10912#issuecomment-899974928


   CC @xhochy I think last time we talked about this you were able to use the undeprecated methods, just  wanted to make sure this is still the case or you have a migration plan.


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



[GitHub] [arrow] emkornfield commented on a change in pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #10912:
URL: https://github.com/apache/arrow/pull/10912#discussion_r690016023



##########
File path: java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/AbstractJdbcToArrowTest.java
##########
@@ -140,4 +146,160 @@ public void destroy() throws SQLException {
    */
   public abstract void testDataSets(VectorSchemaRoot root);
 
+  /**
+   * For the given SQL query, execute and fetch the data from Relational DB and convert it to Arrow objects.
+   * This method uses the default Calendar instance with default TimeZone and Locale as returned by the JVM.
+   * If you wish to use specific TimeZone or Locale for any Date, Time and Timestamp datasets, you may want use
+   * overloaded API that taken Calendar object instance.
+   *
+   * @param connection Database connection to be used. This method will not close the passed connection object. Since
+   *                   the caller has passed the connection object it's the responsibility of the caller to close or
+   *                   return the connection to the pool.
+   * @param query      The DB Query to fetch the data.
+   * @param allocator  Memory allocator
+   * @return Arrow Data Objects {@link VectorSchemaRoot}
+   * @throws SQLException Propagate any SQL Exceptions to the caller after closing any resources opened such as
+   *                      ResultSet and Statement objects.
+   */
+  public static VectorSchemaRoot sqlToArrow(Connection connection, String query, BufferAllocator allocator)

Review comment:
       It seems ashame to simply do a code move here, would it make sense to make the deprecated methods package private instead and commnent that they are test only?
   
   I think this also reminds me that the model for the iteration here doesn't match the Loader/Unloader pattern but creates a new VectorSchemaRoot each time (I might be misremembering though)




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



[GitHub] [arrow] emkornfield commented on pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #10912:
URL: https://github.com/apache/arrow/pull/10912#issuecomment-911185661


   +1 thank you.


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



[GitHub] [arrow] emkornfield closed pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

Posted by GitBox <gi...@apache.org>.
emkornfield closed pull request #10912:
URL: https://github.com/apache/arrow/pull/10912


   


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



[GitHub] [arrow] github-actions[bot] commented on pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10912:
URL: https://github.com/apache/arrow/pull/10912#issuecomment-896610504


   https://issues.apache.org/jira/browse/ARROW-13544


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



[GitHub] [arrow] liyafan82 commented on a change in pull request #10912: ARROW-13544 [Java]: Remove APIs that have been deprecated for long (Changes to JDBC)

Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #10912:
URL: https://github.com/apache/arrow/pull/10912#discussion_r690232354



##########
File path: java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/AbstractJdbcToArrowTest.java
##########
@@ -140,4 +146,160 @@ public void destroy() throws SQLException {
    */
   public abstract void testDataSets(VectorSchemaRoot root);
 
+  /**
+   * For the given SQL query, execute and fetch the data from Relational DB and convert it to Arrow objects.
+   * This method uses the default Calendar instance with default TimeZone and Locale as returned by the JVM.
+   * If you wish to use specific TimeZone or Locale for any Date, Time and Timestamp datasets, you may want use
+   * overloaded API that taken Calendar object instance.
+   *
+   * @param connection Database connection to be used. This method will not close the passed connection object. Since
+   *                   the caller has passed the connection object it's the responsibility of the caller to close or
+   *                   return the connection to the pool.
+   * @param query      The DB Query to fetch the data.
+   * @param allocator  Memory allocator
+   * @return Arrow Data Objects {@link VectorSchemaRoot}
+   * @throws SQLException Propagate any SQL Exceptions to the caller after closing any resources opened such as
+   *                      ResultSet and Statement objects.
+   */
+  public static VectorSchemaRoot sqlToArrow(Connection connection, String query, BufferAllocator allocator)

Review comment:
       > It seems ashame to simply do a code move here, would it make sense to make the deprecated methods package private instead and commnent that they are test only?
   > 
   I think it is a good idea to explicitly state that these methods are for test only. However, it is not feasible to declare them as package private, as they are used in at least two packages: `org.apache.arrow.adapter.jdbc` and `org.apache.arrow.adapter.jdbc.h2`.
   
   > I think this also reminds me that the model for the iteration here doesn't match the Loader/Unloader pattern but creates a new VectorSchemaRoot each time (I might be misremembering though)
   
    I think you are right
   
   




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