Skip to content

Commit 39d815e

Browse files
Fix rollback in auto-commit mode and handle numeric autocommit responses (#1206)
## Summary - **rollback() now throws** `DatabricksTransactionException` when called while the connection is in auto-commit mode (no active transaction), aligning behavior with the JDBC spec - **fetchAutoCommitStateFromServer()** now accepts both `"1"`/`"0"` and `"true"`/`"false"` responses from the `SET AUTOCOMMIT` query, since different server implementations return different formats - Updated E2E and unit tests to reflect the corrected behavior ## Context 1. `connection.rollback()` in auto-commit mode should throw an exception, not silently succeed 2. `SET AUTOCOMMIT` query returns `"1"`/`"0"` rather than `"true"`/`"false"` depending on the server ## Test plan - [x] Unit tests pass: `DatabricksConnectionTest` (42/42 pass) - [ ] E2E tests: `TransactionTests` and `ExplicitTransactionStatementTests` against staging warehouse 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 936d666 commit 39d815e

5 files changed

Lines changed: 145 additions & 23 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
### Updated
88

99
### Fixed
10+
- Fixed `rollback()` to throw `SQLException` when called in auto-commit mode (no active transaction), aligning with JDBC spec. Previously it silently sent a ROLLBACK command to the server.
11+
- Fixed `fetchAutoCommitStateFromServer()` to accept both `"1"`/`"0"` and `"true"`/`"false"` responses from `SET AUTOCOMMIT` query, since different server implementations return different formats.
1012

1113
---
1214
*Note: When making changes, please add your change under the appropriate section

src/main/java/com/databricks/jdbc/api/impl/DatabricksConnection.java

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,22 @@ private boolean fetchAutoCommitStateFromServer() throws SQLException {
252252
ResultSet rs = statement.executeQuery("SET AUTOCOMMIT");
253253

254254
if (rs.next()) {
255-
// The result should contain the value = "true" or "false"
255+
// The result may contain "true"/"false" or "1"/"0" depending on the server
256256
String value = rs.getString(1); // Column 1: value
257257

258258
LOGGER.debug(
259259
"Fetched autoCommit state from server: value={}. Updating session cache.", value);
260260

261-
boolean autoCommitState = "true".equalsIgnoreCase(value);
261+
boolean autoCommitState;
262+
if ("true".equalsIgnoreCase(value) || "1".equals(value)) {
263+
autoCommitState = true;
264+
} else if ("false".equalsIgnoreCase(value) || "0".equals(value)) {
265+
autoCommitState = false;
266+
} else {
267+
throw new DatabricksSQLException(
268+
"Unexpected autoCommit value from server: " + value,
269+
DatabricksDriverErrorCode.TRANSACTION_SET_AUTOCOMMIT_ERROR);
270+
}
262271

263272
// Update the session cache with the server value
264273
session.setAutoCommit(autoCommitState);
@@ -348,24 +357,19 @@ public void commit() throws SQLException {
348357
*
349358
* <ul>
350359
* <li>Rolls back the current transaction
351-
* <li>A new transaction begins automatically (per autocommit design)
360+
* <li>A new transaction begins automatically
352361
* </ul>
353362
*
354363
* <p>When auto-commit is TRUE:
355364
*
356365
* <ul>
357-
* <li>ROLLBACK is a safe no-op (does not throw exception)
358-
* <li>This is more forgiving than COMMIT, which throws an exception when there's no active
359-
* transaction
366+
* <li>This operation throws {@link DatabricksTransactionException} (if ignoreTransactions flag
367+
* is not set)
360368
* </ul>
361369
*
362-
* <p><b>Note:</b> ROLLBACK is designed to be safe to call even when there is no active
363-
* transaction. It can be used to recover from error states without needing to check transaction
364-
* status first.
365-
*
366370
* @throws DatabricksSQLException if the connection is closed
367-
* @throws DatabricksTransactionException for transaction-specific errors (rare - ROLLBACK is
368-
* typically very forgiving)
371+
* @throws DatabricksTransactionException for transaction-specific errors such as
372+
* MULTI_STATEMENT_TRANSACTION_NO_ACTIVE_TRANSACTION or calling rollback in auto-commit mode
369373
* @see #setAutoCommit(boolean)
370374
* @see #commit()
371375
*/
@@ -382,13 +386,20 @@ public void rollback() throws SQLException {
382386
return;
383387
}
384388

389+
// Rollback is not valid when auto-commit is enabled (no active transaction)
390+
if (getAutoCommit()) {
391+
throw new DatabricksTransactionException(
392+
"Cannot use rollback while Connection is in auto-commit mode.",
393+
new SQLException("Cannot use rollback while Connection is in auto-commit mode."),
394+
DatabricksDriverErrorCode.TRANSACTION_ROLLBACK_ERROR);
395+
}
396+
385397
// Execute ROLLBACK command
386398
Statement statement = null;
387399
try {
388400
statement = createStatement();
389401
statement.execute("ROLLBACK");
390402
// Note: Server auto-starts new transaction if autocommit=false
391-
// Note: ROLLBACK is more forgiving - typically succeeds even on unexpected states
392403

393404
} catch (SQLException e) {
394405
LOGGER.error(e, "Error {} while rolling back transaction", e.getMessage());

src/test/java/com/databricks/jdbc/api/impl/DatabricksConnectionTest.java

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.databricks.jdbc.exception.DatabricksSQLException;
1919
import com.databricks.jdbc.exception.DatabricksSQLFeatureNotImplementedException;
2020
import com.databricks.jdbc.exception.DatabricksSQLFeatureNotSupportedException;
21+
import com.databricks.jdbc.exception.DatabricksTransactionException;
2122
import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode;
2223
import java.sql.*;
2324
import java.util.*;
@@ -824,12 +825,42 @@ public void testRollbackSuccess() throws SQLException {
824825
DatabricksConnection spyConnection = spy(connection);
825826
DatabricksStatement mockStatement = mock(DatabricksStatement.class);
826827
doReturn(mockStatement).when(spyConnection).createStatement();
828+
when(mockStatement.execute("SET AUTOCOMMIT = FALSE")).thenReturn(true);
827829
when(mockStatement.execute("ROLLBACK")).thenReturn(true);
828830

831+
// Must set autocommit to false first — rollback is not valid in auto-commit mode
832+
spyConnection.setAutoCommit(false);
829833
spyConnection.rollback();
830834

831835
verify(mockStatement).execute("ROLLBACK");
832-
verify(mockStatement).close();
836+
verify(mockStatement, atLeast(2)).close();
837+
838+
spyConnection.close();
839+
}
840+
841+
@Test
842+
public void testRollbackThrowsWhenAutoCommitEnabled() throws SQLException {
843+
when(databricksClient.createSession(
844+
new Warehouse(WAREHOUSE_ID), CATALOG, SCHEMA, new HashMap<>()))
845+
.thenReturn(IMMUTABLE_SESSION_INFO);
846+
connection = new DatabricksConnection(transactionsEnabledContext, databricksClient);
847+
connection.open();
848+
849+
DatabricksConnection spyConnection = spy(connection);
850+
851+
// Auto-commit is true by default — rollback should throw
852+
assertTrue(spyConnection.getAutoCommit());
853+
854+
DatabricksTransactionException thrown =
855+
assertThrows(DatabricksTransactionException.class, spyConnection::rollback);
856+
857+
assertTrue(
858+
thrown.getMessage().contains("auto-commit"),
859+
"Exception should indicate rollback is not valid in auto-commit mode. Got: "
860+
+ thrown.getMessage());
861+
862+
// Connection should still be usable
863+
assertFalse(spyConnection.isClosed());
833864

834865
spyConnection.close();
835866
}
@@ -865,6 +896,10 @@ public void testRollbackWithServerError() throws SQLException {
865896
DatabricksConnection spyConnection = spy(connection);
866897
DatabricksStatement mockStatement = mock(DatabricksStatement.class);
867898
doReturn(mockStatement).when(spyConnection).createStatement();
899+
when(mockStatement.execute("SET AUTOCOMMIT = FALSE")).thenReturn(true);
900+
901+
// Must set autocommit to false first — rollback is not valid in auto-commit mode
902+
spyConnection.setAutoCommit(false);
868903

869904
SQLException serverError = new SQLException("Unexpected rollback error", "HY000", 99999);
870905
when(mockStatement.execute("ROLLBACK")).thenThrow(serverError);
@@ -875,7 +910,7 @@ public void testRollbackWithServerError() throws SQLException {
875910
assertEquals("HY000", thrown.getSQLState());
876911
assertEquals(99999, thrown.getErrorCode()); // Vendor code preserved
877912

878-
verify(mockStatement).close();
913+
verify(mockStatement, atLeast(2)).close();
879914

880915
spyConnection.close();
881916
}
@@ -986,6 +1021,68 @@ public void testGetAutoCommitWithFetchFromServerEnabled_ReturnsFalse() throws SQ
9861021
spyConnection.close();
9871022
}
9881023

1024+
@Test
1025+
public void testGetAutoCommitWithFetchFromServerEnabled_ReturnsNumeric1() throws SQLException {
1026+
// Server may return "1" instead of "true" for autocommit enabled
1027+
String urlWithFetch = CATALOG_SCHEMA_JDBC_URL + ";FetchAutoCommitFromServer=1";
1028+
IDatabricksConnectionContext contextWithFetch =
1029+
DatabricksConnectionContext.parse(urlWithFetch, new Properties());
1030+
1031+
when(databricksClient.createSession(
1032+
new Warehouse(WAREHOUSE_ID), CATALOG, SCHEMA, new HashMap<>()))
1033+
.thenReturn(IMMUTABLE_SESSION_INFO);
1034+
connection = new DatabricksConnection(contextWithFetch, databricksClient);
1035+
connection.open();
1036+
1037+
DatabricksConnection spyConnection = spy(connection);
1038+
DatabricksStatement mockStatement = mock(DatabricksStatement.class);
1039+
ResultSet mockResultSet = mock(ResultSet.class);
1040+
1041+
doReturn(mockStatement).when(spyConnection).createStatement();
1042+
when(mockStatement.executeQuery("SET AUTOCOMMIT")).thenReturn(mockResultSet);
1043+
when(mockResultSet.next()).thenReturn(true);
1044+
when(mockResultSet.getString(1)).thenReturn("1"); // Server returns "1"
1045+
1046+
boolean result = spyConnection.getAutoCommit();
1047+
1048+
verify(mockStatement).executeQuery("SET AUTOCOMMIT");
1049+
assertTrue(result);
1050+
assertTrue(spyConnection.getSession().getAutoCommit());
1051+
1052+
spyConnection.close();
1053+
}
1054+
1055+
@Test
1056+
public void testGetAutoCommitWithFetchFromServerEnabled_ReturnsNumeric0() throws SQLException {
1057+
// Server may return "0" instead of "false" for autocommit disabled
1058+
String urlWithFetch = CATALOG_SCHEMA_JDBC_URL + ";FetchAutoCommitFromServer=1";
1059+
IDatabricksConnectionContext contextWithFetch =
1060+
DatabricksConnectionContext.parse(urlWithFetch, new Properties());
1061+
1062+
when(databricksClient.createSession(
1063+
new Warehouse(WAREHOUSE_ID), CATALOG, SCHEMA, new HashMap<>()))
1064+
.thenReturn(IMMUTABLE_SESSION_INFO);
1065+
connection = new DatabricksConnection(contextWithFetch, databricksClient);
1066+
connection.open();
1067+
1068+
DatabricksConnection spyConnection = spy(connection);
1069+
DatabricksStatement mockStatement = mock(DatabricksStatement.class);
1070+
ResultSet mockResultSet = mock(ResultSet.class);
1071+
1072+
doReturn(mockStatement).when(spyConnection).createStatement();
1073+
when(mockStatement.executeQuery("SET AUTOCOMMIT")).thenReturn(mockResultSet);
1074+
when(mockResultSet.next()).thenReturn(true);
1075+
when(mockResultSet.getString(1)).thenReturn("0"); // Server returns "0"
1076+
1077+
boolean result = spyConnection.getAutoCommit();
1078+
1079+
verify(mockStatement).executeQuery("SET AUTOCOMMIT");
1080+
assertFalse(result);
1081+
assertFalse(spyConnection.getSession().getAutoCommit());
1082+
1083+
spyConnection.close();
1084+
}
1085+
9891086
@Test
9901087
public void testGetAutoCommitWithFetchFromServerEnabled_ServerQueryFails() throws SQLException {
9911088
// Create connection context with FetchAutoCommitFromServer=1

src/test/java/com/databricks/jdbc/integration/e2e/ExplicitTransactionStatementTests.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,8 +475,11 @@ void testSetAutocommitWithoutValue() throws SQLException {
475475
ResultSet rs1 = stmt.executeQuery("SET AUTOCOMMIT");
476476
assertTrue(rs1.next(), "SET AUTOCOMMIT should return a result");
477477
// The result should indicate autocommit is TRUE (default)
478+
// Note: The server may return "1"/"0" instead of "true"/"false"
478479
String value1 = rs1.getString(1);
479-
assertTrue(Boolean.parseBoolean(value1), "Should return a value");
480+
assertTrue(
481+
"true".equalsIgnoreCase(value1) || "1".equals(value1),
482+
"Default autocommit should be true/1. Got: " + value1);
480483
rs1.close();
481484

482485
// Set autocommit to FALSE

src/test/java/com/databricks/jdbc/integration/e2e/TransactionTests.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,16 +344,25 @@ void testCommitWithoutActiveTransaction() throws SQLException {
344344
}
345345

346346
@Test
347-
@DisplayName("Should safely handle ROLLBACK without active transaction (no-op)")
348-
void testRollbackWithoutActiveTransactionIsNoOp() throws SQLException {
347+
@DisplayName("Should throw exception when rolling back without active transaction")
348+
void testRollbackWithoutActiveTransactionThrows() throws SQLException {
349349
// With autoCommit=true (no active transaction)
350350
assertTrue(connection.getAutoCommit());
351351

352-
// ROLLBACK is more forgiving than COMMIT - it should succeed as a no-op
353-
// when there's no active transaction
354-
assertDoesNotThrow(
355-
() -> connection.rollback(),
356-
"ROLLBACK should be a safe no-op when autocommit=true (no active transaction)");
352+
// ROLLBACK should throw an exception when there is no active transaction
353+
// (connection is in auto-commit mode)
354+
DatabricksTransactionException exception =
355+
assertThrows(
356+
DatabricksTransactionException.class,
357+
() -> connection.rollback(),
358+
"ROLLBACK should throw exception when autocommit=true (no active transaction)");
359+
360+
assertTrue(
361+
exception.getMessage().contains("auto-commit")
362+
|| exception.getMessage().contains("rollback")
363+
|| exception.getMessage().contains("No active transaction"),
364+
"Exception message should indicate rollback is not valid in auto-commit mode. Got: "
365+
+ exception.getMessage());
357366

358367
// Verify connection is still usable
359368
assertTrue(connection.getAutoCommit());

0 commit comments

Comments
 (0)