Repository navigation
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #287.
The rollback added to
BatchedQuery(andBatchedInsert) 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:Reproduction on
develop(Postgres 16, PostgreSQL JDBC 42.3.10):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 privatefetchRows()so the diff stays small.BatchedInsert: switch auto-commit back on after the existing rollback inexecuteBatch()andexecuteBigQueryBatch().inst/java/DatabaseConnector.jar(onlyBatchedQuery.classandBatchedInsert.classreplaced, compiled withjavac --release 8, class file version 52 like the existing jar) and updatedinst/csv/jarChecksum.txt.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 failingexecuteSql()does not break the following query. On Postgres it also covers an error raised in a later fetch batch.Testing:
develop, all pass with this change.testthatsuite with Postgres (Eunomia GiBleed loaded into acdmschema), SQLite and DuckDB: the only remaining failures are the twotest-dbplyr.Rerrors, which happen the same way ondevelop(local data types and the installed bigrquery version).R CMD check --no-manual --ignore-vignettes: Status: OK.NEWS.md is left for the maintainer.