Mark JDBC transaction rollback-only after caught SQL failures

Author: Philip Z

teaql-provider-jdbc/src/main/java/io/teaql/provider/jdbc/JdbcSqlExecutor.java

@@ -184,6 +184,7 @@ public List<Map<String, Object>> queryForList(String sql, Object[] params) {
                 return result;
             }
         } catch (SQLException e) {
+            markTransactionRollbackOnly();
             throw new RuntimeException("JDBC query failed for parameterized SQL: " + sql, e);
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
@@ -210,6 +211,7 @@ public <T extends io.teaql.core.Entity> List<T> query(
             }
             return result;
         } catch (SQLException exception) {
+            markTransactionRollbackOnly();
             throw new RuntimeException("JDBC typed query failed for parameterized SQL: " + sql, exception);
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
@@ -339,6 +341,7 @@ public void execute(String sql) {
             ps.execute();
             }
         } catch (SQLException e) {
+            markTransactionRollbackOnly();
             throw new RuntimeException(e);
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
@@ -364,6 +367,7 @@ public int update(String sql, Object[] params) {
                 return ps.executeUpdate();
             }
         } catch (SQLException e) {
+            markTransactionRollbackOnly();
             throw new RuntimeException(e);
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
@@ -387,6 +391,7 @@ public int[] batchUpdate(String sql, List<Object[]> paramsList) {
                 return ps.executeBatch();
             }
         } catch (SQLException e) {
+            markTransactionRollbackOnly();
             throw new RuntimeException(e);
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
@@ -432,6 +437,13 @@ public void executeInTransaction(Runnable action) {
         }
     }
 
+    private void markTransactionRollbackOnly() {
+        // A failed statement may have left earlier writes in the shared
+        // transaction. Without a savepoint, catching that failure cannot
+        // make the transaction safe to commit.
+        if (transactionConnection.get() != null) transactionRollbackOnly.set(true);
+    }
+
     @Override public boolean hasActiveTransaction() { return transactionConnection.get() != null; }
 
     static void bind(PreparedStatement statement, int index, Object value) throws SQLException {

teaql-provider-jdbc/src/test/java/io/teaql/provider/jdbc/JdbcSqlExecutorTest.java

@@ -91,6 +91,50 @@ public void nestedFailureCaughtByCallerStillRollsBackOuterTransaction() {
                 .get(0).get("total")).intValue());
     }
 
+    @Test
+    public void caughtJdbcWriteFailureStillRollsBackOuterTransaction() {
+        assertThrows(IllegalStateException.class, () -> sqlExecutor.executeInTransaction(() -> {
+            sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                    new Object[] {94, "first write", 20});
+            try {
+                sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                        new Object[] {94, "duplicate key", 21});
+            } catch (RuntimeException expected) {
+                // A caller may handle a low-level JDBC failure before returning.
+            }
+        }));
+
+        assertEquals(0, ((Number) sqlExecutor.queryForList(
+                "SELECT count(*) AS total FROM test_user", new Object[0])
+                .get(0).get("total")).intValue());
+    }
+
+    @Test
+    public void caughtStatementOrReadFailureCannotCommitEarlierWrites() {
+        List<Runnable> failures = List.of(
+                () -> sqlExecutor.execute("INSERT INTO test_user (id, name, age) VALUES (94, 'duplicate', 21)"),
+                () -> sqlExecutor.batchUpdate(
+                        "INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                        java.util.Collections.singletonList(new Object[] {94, "duplicate", 21})),
+                () -> sqlExecutor.queryForList("SELECT * FROM missing_test_table", new Object[0]),
+                () -> sqlExecutor.query("SELECT * FROM missing_test_table", new Object[0],
+                        row -> new TypedRow()));
+        for (Runnable failedStatement : failures) {
+            assertThrows(IllegalStateException.class, () -> sqlExecutor.executeInTransaction(() -> {
+                sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                        new Object[] {94, "must roll back", 20});
+                try {
+                    failedStatement.run();
+                } catch (RuntimeException expected) {
+                    // The failed statement must poison the shared transaction.
+                }
+            }));
+            assertEquals(0, ((Number) sqlExecutor.queryForList(
+                    "SELECT count(*) AS total FROM test_user", new Object[0])
+                    .get(0).get("total")).intValue());
+        }
+    }
+
     @Test
     public void testQueryForListNormalizesAliasesForPortableMapping() {
         List<Map<String, Object>> rows = sqlExecutor.queryForList(