This should set the transaction status to in transaction

[davecramer]

https://github.com/awslabs/aws-advanced-jdbc-wrapper/blob/0b04bc682742e93c07f43c572f9a53fbf77556f6/wrapper/src/main/java/software/amazon/jdbc/wrapper/ConnectionWrapper.java#L684

[karenc-bq]

@davecramer I pushed a fix in #163

[karenc-bq]

I took the change out of #163. We will add the change in a different PR to make sure we considered all the relevant cases

[davecramer]

This should remain open until fixed

[aaron-congo]

Hi @davecramer, there are a couple workflows where setting transaction status based on setAutoCommit is problematic.

1. Switching autocommit values within a transaction. For example, you can start a transaction while autocommit is false, then, while in the same transaction, you can call setAutoCommit(true). In this case, even though autocommit is true, you are still in a transaction
2. While autocommit is on, you can execute "START TRANSACTION". Again, even though autocommit is on, the transaction status should show we are in a transaction until commit is called.

The current implementation for tracking transaction status handles these two scenarios. Admittedly, they seem like strange workflows so whether they follow recommended practices or not is another question. Regardless, since the existing drivers seem to allow this behavior, I believe we should as well.

If that makes sense, let me know and I can close this ticket. Otherwise, let me know if you have any questions. Thanks!

[davecramer]

The spec says this: which suggests otherwise.

NOTE: If this method is called during a transaction and the auto-commit mode is changed, the transaction is committed. If setAutoCommit is called and the auto-commit mode is not changed, the call is a no-op.

[aaron-congo]

Ah okay, thanks for pointing that out, I wasn't aware of that. I'll do some more investigation and get back to you

[davecramer]

So it appears that mysql does indeed do some funky stuff but PostgreSQL doesn't https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
Mysql will actually commit the transaction in flight with another begin (most of the time)

The [BEGIN](https://dev.mysql.com/doc/refman/8.0/en/commit.html) statement differs from the use of the BEGIN keyword that starts a [BEGIN ... END](https://dev.mysql.com/doc/refman/8.0/en/begin-end.html) compound statement. The latter does not cause an implicit commit. See [Section 13.6.1, “BEGIN ... END Compound Statement”](https://dev.mysql.com/doc/refman/8.0/en/begin-end.html).

PostgreSQL will give you a warning saying there is already a transaction in process, but does not commit the transaction in flight.

[aaron-congo]

Hi Dave, could you give me more context regarding your last comment? We were discussing whether setAutoCommit should set inTransaction, but your comment seems to be referring to implicit commits. Just want to make sure I'm understanding.

Also, I did a couple tests with the mysql and postgres community drivers switching autocommit while inside a transaction. Both drivers commit the transaction when switching autocommit from false to true inside the transaction, but neither committed the transaction when switching autocommit from true to false. Should they be committing in this scenario, given the spec you mentioned?

[davecramer]

My last comment is suggesting that it appears there are real differences to the way begin is handled inside a transaction.

As for committing when going from true to false. I don't think there is a transaction that needs to be committed at that point as the are in autocommit mode and every statement is committed.

[aaron-congo]

Gotcha, yeah that's something to keep in mind, thanks!

For committing when going from true to false, what about the case where the user has autocommit on and executes "START TRANSACTION"? I've tested with the mysql and postgres driver and it appears both will let you start a transaction with autocommit on and switch autocommit to false inside the transaction, but neither will commit the transaction in this case

[davecramer]

Well the spec also says you shouldn't do that :)

Note: When configuring a Connection, JDBC applications should use the appropriate Connection method such as setAutoCommit or setTransactionIsolation. Applications should not invoke SQL commands directly to change the connection's configuration when there is a JDBC method available. By default a Connection object is in auto-commit mode, which means that it automatically commits changes after executing each statement. If auto-commit mode has been disabled, the method commit must be called explicitly in order to commit changes; otherwise, database changes will not be saved.

Since the drivers are generally not inspecting SQL they have no way of knowing that you have started a transaction so they also have no way to know whether to commit the transaction. The drivers can only control what happens through the API

[aaron-congo]

Okay thanks, just wanted to verify that executing "START TRANSACTION" is not considered valid according to the spec then? That quote you provided seems to clearly say that, for example, users should not execute "COMMIT" since there is a directly corresponding JDBC method: Connection#commit(). However its less clear to me whether that quote is implying "START TRANSACTION" is not considered valid, since there is not a directly corresponding startTransaction() method. I know setting autocommit false is the usual way of doing things though. Just trying to get clarity, thanks!

[davecramer]

Doing it that way makes it difficult for the driver to know what the state is. It appears that MySQL tracks it. PostgreSQL does not

[aaron-congo]

Right, is "START TRANSACTION" invalid according the JDBC API though? I still can't tell, I can't find anywhere in the spec that clarifies this specific scenario

[davecramer]

it's not "invalid" there's just no specific support for using it.

[aaron-congo]

Right, thanks for the clarification. With this in mind it sounds like the driver does not necessarily have to support tracking transactions for the scenario where the user starts a transaction with "START TRANSACTION" or "BEGIN". Perhaps it might be useful to some customers but it sounds like starting transactions in this way is uncommon (if used at all) and not recommended so we might consider simplifying transaction tracking to exclude tracking this scenario. Does this sound accurate or did I misunderstand?

[davecramer]

I think this is accurately summarizes the situation