Message-ID: From: "vlsi (@vlsi)" To: "pgjdbc/pgjdbc" Date: Mon, 15 Jun 2026 17:38:41 +0000 Subject: [pgjdbc/pgjdbc] PR #4187: fix: avoid AssertionError in BatchResultHandler when the connection is closed List-Id: X-GitHub-Additions: 51 X-GitHub-Author-Id: 213894 X-GitHub-Author-Login: vlsi X-GitHub-Base: master X-GitHub-Changed-Files: 3 X-GitHub-Commits: 1 X-GitHub-Deletions: 2 X-GitHub-Head-Branch: claude/eloquent-sammet-e5d970 X-GitHub-Head-SHA: 632fc37a0292b05b58c98882c36e8eca188a7594 X-GitHub-Issue: 4187 X-GitHub-Labels: bug X-GitHub-Merge-SHA: 41ec61cc8af7e9307394f8d6a78f510eb5b3ff2d X-GitHub-Merged-By: vlsi X-GitHub-Repo: pgjdbc/pgjdbc X-GitHub-State: merged X-GitHub-Type: pull_request X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/4187 Content-Type: text/plain; charset=utf-8 ## 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 issue 1458. + */ + @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); + } + } }