fix: Fix errors on transaction status

[bpvgoncalves]

Some packages dependant on RSQLite are applying commit/rollback when no open transactions exist. Raising this error seems to be breaking some of them.

[krlmlr]

Thanks, good catch. I wonder if we should rather fail issues downstream and release nevertheless. Calling dbCommit() without a transaction should be an error, I wonder why DBItest doesn't test it.

[bpvgoncalves]

I can conceive some reasonable use cases using dbRollback() to try to revert changes to a previous state if some unhandled error occurs, but not really for the dbCommit().

Thinking out loud, maybe a this point replacing the error by a message/warning is a compromise solution and lends them some time to make the necessary changes.

[krlmlr]

A warning sounds about right to me. Thanks!

[bpvgoncalves]

Calling dbCommit() without a transaction should be an error, I wonder why DBItest doesn't test it.

This sounded quite weird for me, and I decided to take a deeper look at their code.
My mistake was I was assuming every dbCommit() or dbRollback() must have a matching dbBegin(). In this case this is not true. They are initiating a transaction by issuing a "BEGIN IMMEDIATE" command via dbExecute and then using dbCommit/dbRollback to close it. This will not trigger an increment on the transaction counter and will then fail when trying to close it.

Let me think a bit more about this to try to find an alternative implementation.

[bpvgoncalves]

Let me think a bit more about this to try to find an alternative implementation.

So, I just completely reworked this. Now we're asking for the transaction status directly to the backend instead of trying to maintain a counter of open transactions. It is much more simple and should also be way more robust.
I do apologize the previous iteration didn't work as intended.

[krlmlr]

Thanks. I wonder if we should test for autocommit mode instead:

• https://stackoverflow.com/a/2219583/946850
• http://www.sqlite.org/c3ref/get_autocommit.html

[bpvgoncalves]

Thanks. I wonder if we should test for autocommit mode instead:

As far as I can see this has the major advantage of not delaying flagging a transaction until a read/write is attempted. Let me take a look at the code, it should not be hard to change the logic to look for autocomnit=off instead of transaction=on.

[krlmlr]

I sent RSQLite 2.3.2 to CRAN, without the new functions. Took me four months to come up with that path of action...

Anyway, once released, we can figure out what to do here, and submit an update.

[bpvgoncalves]

Hello. I'm really sorry and sad the initial implementation didn't quite work as intended and the additional workload it might have caused to you. Please let me know if there is something I can do about it.
I trust the current approach of checking for the autocommit status is much more robust and less error prone than the transaction counters used before.

[krlmlr]

Thanks, no worries. I like the new approach. We can release the update on November 4, the advantage is that we have a CRAN version with SQLite 3.43. I'll run revdepchecks just to be sure.