pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feedFrom: vlsi (@vlsi) <[email protected]>
To: pgjdbc/pgjdbc <[email protected]>
Subject: [pgjdbc/pgjdbc] PR #4187: fix: avoid AssertionError in BatchResultHandler when the connection is closed
Date: Mon, 15 Jun 2026 17:38:41 +0000
Message-ID: <[email protected]> (raw)
## Problem
When the JVM runs with assertions enabled (`-ea`) and the server terminates the connection in the middle of a batch, `executeBatch()` fails with an `AssertionError` instead of a plain `SQLException`:
```
java.lang.AssertionError: pgStatement.getConnection().getAutoCommit() should not throw
at org.postgresql.jdbc.BatchResultHandler.isAutoCommit(BatchResultHandler.java:105)
```
`BatchResultHandler.isAutoCommit()` calls `getConnection().getAutoCommit()`, which runs `checkClosed()` and throws once the connection is gone. The catch block asserted this could never happen, turning a recoverable situation into an `AssertionError` under `-ea`.
This reproduces on both `master` and `release/42.2.x` — the code is identical to the one reported against 42.2.5.
Fixes #1458.
## What changed
- **`BatchResultHandler.isAutoCommit()`**: drop the `assert false`. A closed connection has nothing to commit, so treat it as not auto-committing and let the regular `BatchUpdateException` propagate.
- **`BatchExecuteTest.testBatchOnTerminatedConnection`**: new regression test that terminates the backend mid-batch via `TestUtil.terminateBackend` and asserts a `SQLException` (not an `AssertionError`) is thrown.
- **`.github/workflows/matrix.mjs`**: new `assertions` axis that occasionally adds `-ea` to the test JVM, so CI exercises the driver's `assert` statements (which were never evaluated before).
## Verification
Ran against PostgreSQL in Docker with `-ea`:
- With the fix: all 140 `BatchExecuteTest` cases pass.
- Temporarily restoring `assert false`: the new test fails with `AssertionError: ...getAutoCommit() should not throw`, confirming it catches the regression.
- `matrix.mjs` generates rows tagged `assertions`, and `-ea` lands in `testExtraJvmArgs`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
diff --git a/.github/workflows/matrix.mjs b/.github/workflows/matrix.mjs
index c62824f748..35b00134c7 100644
--- a/.github/workflows/matrix.mjs
+++ b/.github/workflows/matrix.mjs
@@ -257,6 +257,15 @@ matrix.addAxis({
]
});
+matrix.addAxis({
+ name: 'assertions',
+ title: x => x.value === 'yes' ? 'assertions' : '',
+ values: [
+ {value: 'yes', weight: 3},
+ {value: 'no', weight: 10},
+ ]
+});
+
function lessThan(minVersion) {
return value => Number(value) < Number(minVersion);
}
@@ -266,7 +275,7 @@ matrix.setNamePattern([
'server_tz', 'tz', 'locale',
'check_anorm_sbt', 'gss', 'replication', 'slow_tests',
'adaptive_fetch', 'rewrite_batch_inserts', 'query_timeout',
- 'autosave', 'cleanupSavepoints', 'cpu_count'
+ 'autosave', 'cleanupSavepoints', 'cpu_count', 'assertions'
]);
// We take EA builds from Oracle
@@ -317,6 +326,7 @@ matrix.ensureAllAxisValuesCovered('ssl');
matrix.ensureAllAxisValuesCovered('replication');
matrix.ensureAllAxisValuesCovered('os');
matrix.ensureAllAxisValuesCovered('cpu_count');
+matrix.ensureAllAxisValuesCovered('assertions');
// Ensure at least one job with autosave=always
matrix.generateRow({autosave: {value: 'always'}});
const include = matrix.generateRows(process.env.MATRIX_JOBS || 5);
@@ -451,6 +461,10 @@ include.forEach(v => {
testJvmArgs.push('-XX:ActiveProcessorCount=1');
}
delete v.cpu_count;
+ if (v.assertions.value === 'yes') {
+ testJvmArgs.push('-ea');
+ }
+ delete v.assertions;
v.extraJvmArgs = jvmArgs.join(' ');
v.testExtraJvmArgs = testJvmArgs.join(' ::: ');
delete v.hash;
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/BatchResultHandler.java b/pgjdbc/src/main/java/org/postgresql/jdbc/BatchResultHandler.java
index 42642b3a0e..b49a78abbc 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/BatchResultHandler.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/BatchResultHandler.java
@@ -112,7 +112,9 @@ private boolean isAutoCommit() {
try {
return pgStatement.getConnection().getAutoCommit();
} catch (SQLException e) {
- assert false : "pgStatement.getConnection().getAutoCommit() should not throw";
+ // getAutoCommit() throws when the connection is already closed (for instance, the server
+ // terminated the connection in the middle of a batch). There is nothing to commit in that
+ // case, so treat the connection as not auto-committing rather than failing with an assertion.
return false;
}
}
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/BatchExecuteTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/BatchExecuteTest.java
index ec0b62dbd5..ee81d0a303 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/BatchExecuteTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/BatchExecuteTest.java
@@ -12,9 +12,11 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
import org.postgresql.PGProperty;
import org.postgresql.PGStatement;
+import org.postgresql.core.ServerVersion;
import org.postgresql.test.TestUtil;
import org.postgresql.util.PSQLState;
@@ -1387,4 +1389,35 @@ public void testNoServerPrepareOneRow() throws SQLException {
TestUtil.closeQuietly(ps);
}
}
+
+ /**
+ * When the server terminates the connection in the middle of a batch, executeBatch should report a
+ * plain SQLException rather than failing with an AssertionError when assertions are enabled.
+ * See <a href="https://github.com/pgjdbc/pgjdbc/issues/1458">issue 1458</a>.
+ */
+ @Test
+ public void testBatchOnTerminatedConnection() throws Exception {
+ assumeTrue(TestUtil.haveMinimumServerVersion(con, ServerVersion.v8_4),
+ "pg_terminate_backend(...) requires PostgreSQL 8.4+");
+
+ try (PreparedStatement ps = con.prepareStatement("INSERT INTO prep(a) VALUES (?)")) {
+ ps.setInt(1, 1);
+ ps.addBatch();
+ ps.setInt(1, 2);
+ ps.addBatch();
+
+ // Kill the backend so that executeBatch operates on a connection that is already gone.
+ TestUtil.terminateBackend(con);
+
+ // The driver must surface a SQLException; it must not throw an AssertionError even when the
+ // JVM runs with -ea (see BatchResultHandler.isAutoCommit).
+ assertThrows(SQLException.class, ps::executeBatch,
+ "executeBatch on a terminated connection should throw SQLException");
+ } finally {
+ // The connection is gone, so re-open a fresh one for tearDown to clean up against.
+ TestUtil.closeDB(con);
+ con = TestUtil.openDB();
+ con.setAutoCommit(false);
+ }
+ }
}
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: github://pgjdbc/pgjdbc
Cc: [email protected], [email protected]
Subject: Re: [pgjdbc/pgjdbc] PR #4187: fix: avoid AssertionError in BatchResultHandler when the connection is closed
In-Reply-To: <<[email protected]>>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox