Skip to content

Restore auto-commit after failed queries and inserts on JDBC connections - #345

Open
DG47 wants to merge 1 commit into
OHDSI:developfrom
DG47:fix/rollback-restore-autocommit
Open

DG47 wants to merge 1 commit into
OHDSI:developfrom
DG47:fix/rollback-restore-autocommit

Conversation

@DG47

@DG47 DG47 commented Oct 14, 2026

Copy link
Copy Markdown

Closes #287.

The rollback added to BatchedQuery (and BatchedInsert) in v7 ends the failed transaction, but auto-commit is left switched off afterwards. From then on the connection runs inside an open transaction, so the next failing statement aborts it and every following statement on Postgres fails with:

org.postgresql.util.PSQLException: ERROR: current transaction is aborted, commands ignored until end of transaction block

Reproduction on develop (Postgres 16, PostgreSQL JDBC 42.3.10):

con <- connect(connectionDetails)
try(querySql(con, "SELECT * FROM no_such_table;"))
rJava::.jcall(con@jConnection, "Z", "getAutoCommit")
#> [1] FALSE

DBI::dbExecute(con, "CREATE TABLE t1 (x INT)")   # never committed
try(executeSql(con, "INSERT INTO no_such_table VALUES (1);"))
executeSql(con, "CREATE TABLE t2 (x INT);")
#> Error: ... current transaction is aborted, commands ignored until end of transaction block
querySql(con, "SELECT COUNT(*) FROM t1;")
#> Error: ... current transaction is aborted, commands ignored until end of transaction block
querySql(con, "SELECT COUNT(*) FROM t1;")
#> Error: ... relation "t1" does not exist

The same state is left behind by a failed insertTable() (for example a NOT NULL violation), and by an error raised while fetching a later batch of a query result. That last case did not roll back at all.

Changes:

  • BatchedQuery: on an error, either when executing the query or while fetching a batch, roll back (only when auto-commit is off) and switch auto-commit back on. The fetch loop is moved unchanged into a private fetchRows() so the diff stays small.
  • BatchedInsert: switch auto-commit back on after the existing rollback in executeBatch() and executeBigQueryBatch().
  • Rebuilt inst/java/DatabaseConnector.jar (only BatchedQuery.class and BatchedInsert.class replaced, compiled with javac --release 8, class file version 52 like the existing jar) and updated inst/csv/jarChecksum.txt.
  • New tests/testthat/test-errorRecovery.R. For every test server it checks that the connection still works after a failed query and after a failed insert: auto-commit is back on, and a failing executeSql() does not break the following query. On Postgres it also covers an error raised in a later fetch batch.

Testing:

  • New tests against a local Postgres 16 container plus SQLite and DuckDB: 4 failures on develop, all pass with this change.
  • Full testthat suite with Postgres (Eunomia GiBleed loaded into a cdm schema), SQLite and DuckDB: the only remaining failures are the two test-dbplyr.R errors, which happen the same way on develop (local data types and the installed bigrquery version).
  • R CMD check --no-manual --ignore-vignettes: Status: OK.

NEWS.md is left for the maintainer.

BatchedQuery and BatchedInsert switch auto-commit off and roll back when a
statement fails, but did not switch auto-commit back on. The connection was
left inside a transaction, so a later failing statement (for example in
executeSql()) aborted it and every subsequent statement on Postgres failed
with 'current transaction is aborted, commands ignored until end of
transaction block'. Statements sent with DBI::dbExecute() after the failure
were also never committed.

Errors raised while fetching later batches of a query result are now rolled
back as well, and the JAR and its checksum are rebuilt (Java 8 target).

Closes OHDSI#287

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant