Visitar URL original
fix(bigquery-jdbc): avoid rollback on statement close in manual commi… · googleapis/google-cloud-java@e2c372a · GitHub
Skip to content

Commit e2c372a

Browse files
yagudin10logachev
authored and
lqiu96
committed
fix(bigquery-jdbc): avoid rollback on statement close in manual commit mode (#13503)
**Problem** Manual commit mode does not work correctly because statement cancellation triggers a transaction rollback. **STR (Steps to Reproduce)** 1. Create a JDBC connection. 2. Disable auto-commit: ```java conn.setAutoCommit(false); ``` 3. Execute a DML statement using a `PreparedStatement`. 4. Close the statement. 5. Call: ```java conn.commit(); ``` **ER (Expected Result)** * Closing a statement does not affect the active transaction. * Pending changes remain available until an explicit `commit()` or `rollback()` is performed on the connection. * `conn.commit()` successfully persists transaction changes. **AR (Actual Result)** * Statement cleanup invokes `cancel()`. * `cancel()` performs a transaction rollback. * Pending transaction changes are discarded before `conn.commit()` is called. * `conn.commit()` has no effect because the transaction has already been rolled back. **Sample Java Code:** ```java try (Connection conn = DriverManager.getConnection(jdbcUrl, props)) { conn.setAutoCommit(false); try (PreparedStatement ps = conn.prepareStatement( "INSERT INTO test_table(id) VALUES (1)")) { ps.executeUpdate(); } // statement cleanup invokes cancel() -> rollback conn.commit(); // nothing is committed } ``` **Fix** Remove transaction rollback from the statement cancellation path. Transaction state is now managed exclusively through explicit `Connection.commit()` and `Connection.rollback()` operations. --------- Co-authored-by: Kirill Logachev <kirl@google.com>
1 parent ecaaceb commit e2c372a

4 files changed

Lines changed: 119 additions & 3 deletions

File tree

‎java-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryConnection.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,21 @@ private void closeImpl() throws SQLException {
992992
}
993993
this.openStatements.clear();
994994

995+
if (isTransactionStarted()) {
996+
try {
997+
// It looks like there's no need to start a new transaction after a rollback,
998+
// but the commit behavior is preserved since close() may still fail before isClosed is
999+
// updated.
1000+
rollbackImpl();
1001+
} catch (SQLException e) {
1002+
if (exceptionToThrow == null) {
1003+
exceptionToThrow = e;
1004+
} else {
1005+
exceptionToThrow.addSuppressed(e);
1006+
}
1007+
}
1008+
}
1009+
9951010
boolean interrupted = Thread.currentThread().isInterrupted();
9961011

9971012
try {

‎java-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryStatement.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -526,9 +526,6 @@ private void closeStatementResources() throws SQLException {
526526
this.currentUpdateCount = -1;
527527
this.currentJobIdIndex = -1;
528528
if (this.connection != null) {
529-
if (this.connection.isTransactionStarted()) {
530-
this.connection.rollback();
531-
}
532529
this.connection.removeStatement(this);
533530
}
534531
}

‎java-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/BigQueryStatementTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,20 @@ public void testCancelWithJoblessQuery() throws SQLException, InterruptedExcepti
492492
verify(bigquery, Mockito.never()).cancel(any(JobId.class));
493493
}
494494

495+
@Test
496+
public void testCancelDoesNotRollbackTransaction() throws SQLException {
497+
doReturn(true).when(bigQueryConnection).isTransactionStarted();
498+
BigQueryStatement statementSpy = Mockito.spy(bigQueryStatement);
499+
statementSpy.jobIds.add(jobId);
500+
501+
statementSpy.cancel();
502+
503+
// Cancel should call bigquery.cancel() but not rollback the transaction
504+
verify(bigquery).cancel(eq(jobId));
505+
verify(bigQueryConnection, Mockito.never()).rollback();
506+
verify(bigQueryConnection).removeStatement(statementSpy);
507+
}
508+
495509
@ParameterizedTest
496510
@ValueSource(booleans = {true, false})
497511
public void testGetStatementType(boolean isReadOnlyTokenUsed) throws Exception {

‎java-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITBigQueryJDBCTest.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2211,6 +2211,96 @@ public void testConnectionWithMultipleTransactionCommits() throws SQLException {
22112211
connection.close();
22122212
}
22132213

2214+
@Test
2215+
public void testPreparedStatementCloseDoesNotRollbackTransaction() throws SQLException {
2216+
String TRANSACTION_TABLE = "JDBC_PS_CLOSE_TABLE" + randomNumber;
2217+
String createTransactionTable =
2218+
String.format(
2219+
"CREATE OR REPLACE TABLE %s.%s (`id` INTEGER, `name` STRING, `age` INTEGER);",
2220+
DATASET, TRANSACTION_TABLE);
2221+
String insertQuery =
2222+
String.format(
2223+
"INSERT INTO %s.%s (id, name, age) VALUES (?, ?, ?);", DATASET, TRANSACTION_TABLE);
2224+
String selectQuery =
2225+
String.format("SELECT id, name, age FROM %s.%s ORDER BY id;", DATASET, TRANSACTION_TABLE);
2226+
2227+
bigQueryStatement.execute(createTransactionTable);
2228+
2229+
try (Connection connection = DriverManager.getConnection(session_enabled_connection_uri)) {
2230+
connection.setAutoCommit(false);
2231+
try (PreparedStatement ps1 = connection.prepareStatement(insertQuery);
2232+
PreparedStatement ps2 = connection.prepareStatement(insertQuery)) {
2233+
ps1.setInt(1, 1);
2234+
ps1.setString(2, "DwightShrute");
2235+
ps1.setInt(3, 10);
2236+
assertEquals(1, ps1.executeUpdate());
2237+
2238+
ps2.setInt(1, 2);
2239+
ps2.setString(2, "MichaelScott");
2240+
ps2.setInt(3, 20);
2241+
assertEquals(1, ps2.executeUpdate());
2242+
2243+
ps1.close();
2244+
connection.commit();
2245+
2246+
try (ResultSet resultSet = bigQueryStatement.executeQuery(selectQuery)) {
2247+
int rowCount = 0;
2248+
while (resultSet.next()) {
2249+
rowCount++;
2250+
assertEquals(rowCount, resultSet.getInt(1));
2251+
}
2252+
assertEquals(2, rowCount);
2253+
}
2254+
} finally {
2255+
bigQueryStatement.execute(
2256+
String.format("DROP TABLE IF EXISTS %s.%s", DATASET, TRANSACTION_TABLE));
2257+
}
2258+
}
2259+
}
2260+
2261+
@Test
2262+
public void testClosingUnusedPreparedStatementDoesNotRollbackPreviousExecute()
2263+
throws SQLException {
2264+
String TRANSACTION_TABLE = "JDBC_PS_UNUSED_CLOSE_TABLE" + randomNumber;
2265+
String createTransactionTable =
2266+
String.format(
2267+
"CREATE OR REPLACE TABLE %s.%s (`id` INTEGER, `name` STRING, `age` INTEGER);",
2268+
DATASET, TRANSACTION_TABLE);
2269+
String insertQuery =
2270+
String.format(
2271+
"INSERT INTO %s.%s (id, name, age) VALUES (?, ?, ?);", DATASET, TRANSACTION_TABLE);
2272+
String selectQuery =
2273+
String.format("SELECT id, name, age FROM %s.%s ORDER BY id;", DATASET, TRANSACTION_TABLE);
2274+
2275+
bigQueryStatement.execute(createTransactionTable);
2276+
2277+
try (Connection connection = DriverManager.getConnection(session_enabled_connection_uri)) {
2278+
connection.setAutoCommit(false);
2279+
try (PreparedStatement ps1 = connection.prepareStatement(insertQuery);
2280+
PreparedStatement ps2 = connection.prepareStatement(insertQuery)) {
2281+
2282+
ps2.setInt(1, 1);
2283+
ps2.setString(2, "MichaelScott");
2284+
ps2.setInt(3, 20);
2285+
assertEquals(1, ps2.executeUpdate());
2286+
2287+
ps1.close();
2288+
connection.commit();
2289+
2290+
try (ResultSet resultSet = bigQueryStatement.executeQuery(selectQuery)) {
2291+
assertTrue(resultSet.next());
2292+
assertEquals(1, resultSet.getInt(1));
2293+
assertEquals("MichaelScott", resultSet.getString(2));
2294+
assertEquals(20, resultSet.getInt(3));
2295+
assertFalse(resultSet.next());
2296+
}
2297+
}
2298+
} finally {
2299+
bigQueryStatement.execute(
2300+
String.format("DROP TABLE IF EXISTS %s.%s", DATASET, TRANSACTION_TABLE));
2301+
}
2302+
}
2303+
22142304
// Private Helper functions
22152305
private String getSessionId() throws InterruptedException {
22162306
QueryJobConfiguration stubJobConfig =

0 commit comments

Comments
 (0)