Fix failure with enabling JDBC autocommit#23453
Merged
tdcmeehan merged 1 commit intoprestodb:masterfrom Aug 16, 2024
Merged
Conversation
Contributor
|
Just a nit to add the PR number to release note entry. |
rschlussel
force-pushed
the
fix-autocommit
branch
from
1d94a47 to
f17a7e6
Compare
rschlussel
commented
Aug 15, 2024
| connection.setAutoCommit(false); | ||
| try (Statement statement = connection.createStatement()) { | ||
| statement.execute("CREATE TABLE test_commit (x bigint)"); | ||
| try { |
Contributor
Author
There was a problem hiding this comment.
It's hard to see in github, but the only thing that's changed about this test is that it's wrapped in an extra try so I could put a finally at the end to drop the table.
Contributor
There was a problem hiding this comment.
A neat trick is to take the url in the browser and add the query parameter ?w=1, which will cause whitespaces to be ignored. e.g. https://github.com/prestodb/presto/pull/23453/files?w=1
tdcmeehan
previously approved these changes
Aug 15, 2024
presto-jdbc/src/test/java/com/facebook/presto/jdbc/TestJdbcConnection.java
Outdated
Show resolved
Hide resolved
There was a bug where enabling autocommit when it had previously been false would cause failure like: java.sql.SQLException: Connection is in auto-commit mode This was due to calling commit() from setAutoCommit() after autocommit had already been changed to true. Fixes prestodb#15916
rschlussel
force-pushed
the
fix-autocommit
branch
from
f17a7e6 to
38d8a5d
Compare
hantangwangd
approved these changes
Aug 16, 2024
Member
hantangwangd
left a comment
hantangwangd
left a comment
There was a problem hiding this comment.
Thanks for the quick fix.
ZacBlanco
approved these changes
Aug 16, 2024
tdcmeehan
approved these changes
Aug 16, 2024
34 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
There was a bug where enabling autocommit when it had previously been false would cause failure like:
java.sql.SQLException: Connection is in auto-commit mode
This was due to calling commit() from setAutoCommit() after autocommit had already been changed to true.
Motivation and Context
Fixes #15916
Impact
Fixes a failure when using JDBC when setting AutoCommit from false to true
Test Plan
unit test
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.