-
Notifications
You must be signed in to change notification settings - Fork 108
GH-942: Fix JDBC Connection.setCatalog() #943
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
Conversation
This comment has been minimized.
This comment has been minimized.
|
Apologies for the larger than needed PR. I couldn't get the tests passing without running spotless:apply and for some reason that reformatted parts that I did not even touch. |
|
CC @jbonofre @laurentgo any comments? |
| statementHandlePreparedStatementMap.remove(new StatementHandleKey(statementHandle)); | ||
| // Testing if the prepared statement was created because the statement can be not created until | ||
| // Testing if the prepared statement was created because the statement can be | ||
| // not created until |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this change is unecessary (I guess it's due to reformating)
| return new ExecuteResult(Collections.singletonList(metaResultSet)); | ||
| } catch (SQLTimeoutException e) { | ||
| // So far AvaticaStatement(executeInternal) only handles NoSuchStatement and Runtime | ||
| // So far AvaticaStatement(executeInternal) only handles NoSuchStatement and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same here
|
LGTM, I'm merging. |
What's Changed
Connection.setCatalog() is not silently ignored anymore (through the default implementation in Calcite) but instead it updates the catalog session option in the same way as during the initial connection.
Closes #942.