Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FlightRPC][C++][Java] Fix example Flight SQL servers #32491

Closed
asfimport opened this issue Jul 25, 2022 · 4 comments
Closed

[FlightRPC][C++][Java] Fix example Flight SQL servers #32491

asfimport opened this issue Jul 25, 2022 · 4 comments

Comments

@asfimport
Copy link
Collaborator

asfimport commented Jul 25, 2022

There are a number of small bugs in the Java Flight SQL example (e.g. binding parameters to the wrong index, not handling null parameter values, not properly reporting errors, things throwing SQLException that should use Arrow/general Java exceptions) that should be fixed.

Also, CommandPreparedStatementQuery is implemented wrong in both C++ and Java servers: only the last parameter value will be bound/executed when executing a prepared statement that returns a result set. But this contradicts the spec, which states "All of the bound parameter sets will be executed as a single atomic execution". The server(s) need to cache the uploaded Arrow data and execute using all input rows.

Reporter: David Li / @lidavidm
Assignee: David Li / @lidavidm

Related issues:

PRs and other links:

Note: This issue was originally created as ARROW-17199. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

David Li / @lidavidm:
CC @jduo: is the behavior for CommandPreparedStatementQuery here expected? Right now, in DoPut, the Java/C++ servers bind each row successively, but don't actually execute anything. So when you go to actually execute the query later, you get only one result row (corresponding to the last parameter row).

@asfimport
Copy link
Collaborator Author

James Duong / @jduo:
The intent for using DoPut to bind parameters was more geared around using parameters for write – it models the behavior of executing a write with an array of parameters an ODBC or PreparedStatement.executeBatch() in JDBC.

For the query case the spec is left open as this iteration of the spec did not include support for multiple result sets.

@asfimport
Copy link
Collaborator Author

David Li / @lidavidm:
Thanks - I'll start a discussion and probably roll it up into the existing proposal, since this would be potentially useful to cover

@asfimport
Copy link
Collaborator Author

David Li / @lidavidm:
Issue resolved by pull request 13710
#13710

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants