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.

Sign up for GitHub

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

Jump to bottom

[Java] JDBC Flight SQL driver calls prepare() twice #34284

Closed
lidavidm opened this issue Feb 22, 2023 · 1 comment · Fixed by #34358
Closed

[Java] JDBC Flight SQL driver calls prepare() twice #34284

lidavidm opened this issue Feb 22, 2023 · 1 comment · Fixed by #34358
Assignees
@rtadepalli
Labels
Component: FlightRPC Component: Java Type: bug
Milestone
12.0.0

Comments

@lidavidm
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

From user@:

I noticed that when I use ArrowFlightJdbcDriver to run a prepare statement
query. It sends prepare query twice to the backend flight server.

The first query is triggered in ArrowFlightMetaImpl#prepare method: link:

arrow/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightMetaImpl.java

Line 134 in 45918a9

public StatementHandle prepare(final ConnectionHandle connectionHandle,

The second query is triggered
in ArrowFlightJdbcFactory#newPreparedStatement: link

arrow/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFactory.java

Line 84 in 45918a9

return ArrowFlightPreparedStatement.createNewPreparedStatement(

Could you shed some light on this? Why it's called twice? Does backend
flight server need skip the second duplicate query in such case?

Component(s)

FlightRPC, Java
@github-actions github-actions bot mentioned this issue Feb 27, 2023
Merged
@rtadepalli
Copy link
Contributor

take

lidavidm pushed a commit that referenced this issue Feb 28, 2023
@rtadepalli
GH-34284: [Java][FlightRPC] Fixed issue with prepared statement getti…
ef21008 
…ng sent twice (#34358)

### Rationale for this change

The `AvaticaConnection` class within the Calcite library [executes](https://github.com/apache/calcite-avatica/blob/b57eb7cd31a90d3f46b65c13832b398be5b0dad9/core/src/main/java/org/apache/calcite/avatica/AvaticaConnection.java#L357) the `prepareStatement(...)` function on a DB connection in the following order:
1. Calls `prepare()` on the `MetaImpl` class, which in our case is `ArrowFlightMetaImpl`. This function executes a command and returns a prepared statement.
2. Calls `newPreparedStatement` on the JDBC factory class, which in our case is `ArrowFlightJdbcFactory`. This function today creates an entirely new `PreparedStatement` without checking the statement cache, thereby causing 2 executions.

This PR fixes the issue by checking whether or not the `StatementHandle` has been recorded before, and if so, uses the results of the previously executed command. If not, it creates a brand new `PreparedStatement`.

My understanding is that `ConcurrentHashMap` (which is what is being used to keep a mapping of statement handles to prepared statements) is eventually consistent, so there _may_ be cases where the prepared statement execution runs twice.

### What changes are included in this PR?
Check to see if the `PreparedStatement` has run before, and if so, use that instead of executing the command to contact the server again.

### Are these changes tested?

Existing tests should cover this case. Namely, `ArrowFlightPreparedStatementTest` I _think_ should cover all cases.

### Are there any user-facing changes?
There are no user facing changes.
* Closes: #34284

Authored-by: Ramasai <ramasai.tadepalli+3108@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 12.0.0 milestone Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rtadepalli rtadepalli

Labels
Component: FlightRPC Component: Java Type: bug
Projects
None yet
Milestone
12.0.0
Development

Successfully merging a pull request may close this issue.

GH-34284: [Java][FlightRPC] Fixed issue with prepared statement getting sent twice rtadepalli/arrow
2 participants
@lidavidm @rtadepalli