AssertionError in BatchResultHandler.isAutoCommit()

[sa7msw]

In complete happenstance, an unexpected AssertionError was thrown by BatchResultHandler.isAutoCommit() after the occurrence of a previous unknown error - the AssertionError propagating out the stack swallowed the original exception and no clue was left behind to know what that was. The problem here is either a false assertion, or the calling code should be a lot more careful about when isAutoCommit() is invoked.

Here is the relevant stack trace:

java.lang.AssertionError: pgStatement.getConnection().getAutoCommit() should not throw
        at org.postgresql.jdbc.BatchResultHandler.isAutoCommit(BatchResultHandler.java:115)
        at org.postgresql.jdbc.BatchResultHandler.handleCompletion(BatchResultHandler.java:179)
        at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:585)
        at org.postgresql.jdbc.PgStatement.internalExecuteBatch(PgStatement.java:896)
        at org.postgresql.jdbc.PgStatement.executeBatch(PgStatement.java:919)
        at org.postgresql.jdbc.PgPreparedStatement.executeBatch(PgPreparedStatement.java:1685)
        ...

isAutoCommit() looks like this:

  private boolean isAutoCommit() {
    try {
      return pgStatement.getConnection().getAutoCommit();
    } catch (SQLException e) {
      assert false : "pgStatement.getConnection().getAutoCommit() should not throw";
      return false;
    }
  }

The javadoc for getAutoCommit() says an SQLException is thrown:

if a database access error occurs or this method is called on a closed connection

However this method is called within BatchResultHandler from a code path already handling an exceptional situation:

175   public void handleCompletion() throws SQLException {
176     updateGeneratedKeys();
177     SQLException batchException = getException();
178     if (batchException != null) {
179       if (isAutoCommit()) {
...

Presumably what happened here is that the batch update failed (for an unknown reason) with an SQLException, which closed the connection, then the code handling that exception called a method that expects the connection to still be open.

Either the assertion in isAutoCommit() should be removed, or handleCompletion() should be improved to only call isAutoCommit() if there's some guarantee that it's not going to fail the assert.

Driver version: 42.6.0. I've checked the code in the latest release (42.7.3 as of this writing) and it's still identical.
Java version: Microsoft OpenJDK 64-Bit Server VM 11.0.15+10-LTS
OS version: RHEL 8.8
PostgreSQL version: 12.18

This issue is not reproducible. Looking at git blame the BatchResultHandler code in question here hasn't changed in eight years, and this is the first time we've see the issue. (We've been using various versions of this driver for the last two decades.) There is nothing at all in the PostgreSQL logs to indicate what might have tripped the original exception, except for this one line that occurred some seconds after the AssertionError:

< 2024-03-16 20:46:44.618 UTC > LOG:  unexpected EOF on client connection with an open transaction

The expected behaviour here of course is that the AssertionError does not occur and the original SQLException that caused this code to be executed be allowed to propagate out instead.

A workaround would be to disable JVM assertions for at least this particular class.

[vlsi]

Ideally the code should not use isAutoCommit, and it should infer the status of the rows (committed or in-flight) based on the completed commit statements