Skip to content

Conversation

@liyafan82
Copy link
Contributor

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

@github-actions
Copy link

* @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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@emkornfield
Copy link
Contributor

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.

@emkornfield
Copy link
Contributor

+1 thank you.

ViniciusSouzaRoque pushed a commit to s1mbi0se/arrow that referenced this pull request Oct 20, 2021
…hanges to JDBC)

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

According to the discussion in apache#10864 (comment), we want to split the task into multiple parts.

This PR is for the changes related to the JDBC adapter

Closes apache#10912 from liyafan82/fly_0811_depj

Authored-by: liyafan82 <fan_li_ya@foxmail.com>
Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…hanges to JDBC)

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

According to the discussion in apache#10864 (comment), we want to split the task into multiple parts.

This PR is for the changes related to the JDBC adapter

Closes apache#10912 from liyafan82/fly_0811_depj

Authored-by: liyafan82 <fan_li_ya@foxmail.com>
Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants