Transaction setReadOnly

[nPraml]

Hello @rbygrave ,

we have the following use case: in some cases we would like to allow the user to start a so-called test run, e.g. updating the user data using external LDAP. The test run does not perform any writing operations in the database, but returns a CSV file of what the updated data would look like. The user can then look at this CSV to see whether the update would be correct and whether they still need to tweak a few parameters of the LDAP import.

The use case runs in a transaction that could potentially be quite large. For this reason, we don't want to rely on a transaction rollback operation. We don't want to put unnecessary load on the database (our experience shows that this is particularly bad with too big DB2 transaction logs).

We tried transaction.setReadOnly(true) which did not reflect our expected results. We thought that if the transaction is in read-only mode and a write operation is called (update/delete), an exception will be thrown and the further process will be aborted.

I have developed a unit test and a possible solution proposal for this. Can you please take a look if it would go in the right direction?

We have chosen a central location that is called for updates/deletes: markNotQueryOnly. Here I queried the local variable because most connections (e.g. H2) don't notice this and connection.isReadOnly returns the wrong result.

Of course, as I receive feedback, I can create more tests and consider other implementations of the markNotQueryOnly.

What else we tried: we create the transaction with readonly scope as try (Transaction txn = DB.beginTransaction(TxScope.required().setReadOnly(true))) { .... It works for a single tenant mode (such as the unittests) but it does not work in a multi tenant mode.

Cheers,
Noemi

[rbygrave]

We thought that if the transaction is in read-only mode and a write operation is called (update/delete), an exception will be thrown and the further process will be aborted.

transaction.setReadOnly(true) does set the underlying java.sql.Connection.setReadOnly(true). So are we saying that DB2 ignores that and an UPDATE was executed and a COMMIT was successful? Or do you mean that it did actually throw an exception but that the exception was thrown later than expected (like at commit() time)?

[rbygrave]

I have developed a unit test and a possible solution proposal for this

This PR looks good. I'm happy with the test and change.

[nPraml]

transaction.setReadOnly(true) does set the underlying java.sql.Connection.setReadOnly(true). So are we saying that DB2 ignores that and an UPDATE was executed and a COMMIT was successful? Or do you mean that it did actually throw an exception but that the exception was thrown later than expected (like at commit() time)?

yes, some databases execute the COMMIT successful.

I modified the test like this and tried it locally with a few database platforms:

  @Test
  public void testReadOnlyTransactionWithUpdateQuery() {
    ResetBasicData.reset();
    try (Transaction txn = DB.beginTransaction()) {
      txn.setReadOnly(true);
      DB.update(Customer.class).set("name", "Rob2").where().eq("name", "Rob").update();
      txn.commit();
    }
    assertThat(DB.find(Customer.class).where().eq("name", "Rob2").exists()).isTrue();
  }

h2, db2, mariadb and sqlserver19 execute the update query despite java.sql.Connection.setReadOnly(true) (and the modified test passes).
Sqlserver19 and h2 don't even notice that they have a readonly connection: java.sql.Connection.isReadOnly() returns false (I checked it by debugging).

Exception flies in mysql:
java.sql.SQLException: Connection is read-only. Queries leading to data modification are not allowed at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:129)

and for postgres:
org.postgresql.util.PSQLException: ERROR: cannot execute UPDATE in a read-only transaction at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2725)

(I haven't tested the other platforms)

[nPraml]

This PR looks good. I'm happy with the test and change.

@rbygrave I wrote a few more tests and added fixes in other places. Can you please review again and give me feedback?

[rbygrave]

Thanks. I made a change where I moved the markNotQueryOnly() into the initTransIfRequired(); and initTransIfRequiredWithBatchCascade() methods. I felt that this was just slightly a better in case any future use of those methods gets added.

I couldn't push that change to this branch so I've merged this into master now with that small adjustment.

Great work, thanks !!

[rbygrave]

h2, db2, mariadb and sqlserver19

I was expecting db2, mariadb and sqlserver to do better there - rather surprising. Thanks for adding those details.