Microsoft.Data.Sqlite: Add deferred transactions

[AlexanderTaeschner]

Adds BeginTransaction(bool deferred) to SqliteConnection.

Fixes #20786

[bricelam]

LGTM

[bricelam]

Hmm, test fails on SQLCipher with no such table: Data during connection1.ExecuteScalar

[AlexanderTaeschner]

On my local build the test works fine for e_sqlcipher, so I'm not sure how to fix it.

[bricelam]

It looks like it passed several times on the CI as well. I'll investigate

[bricelam]

(Rebased and squashed on top of main)

[bricelam]

If it passes this time, I'm inclined to just merge this and deal with the test when/if it becomes flakey again.

[ghost]

Hello @bricelam!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[bricelam]

😒 The flakey test failed on winsqlite3 this time. I'm going to experiment a bit in #22129

[p-m-j]

Would this not make more sense as the default behaviour? If it were it would then align more with the SQLite documentation.

https://www.sqlite.org/lang_transaction.html
2.2. DEFERRED, IMMEDIATE, and EXCLUSIVE transactions
Transactions can be DEFERRED, IMMEDIATE, or EXCLUSIVE. The default transaction behavior is DEFERRED.

https://www.sqlite.org/sharedcache.html#table_level_locking
2.1. Transaction Level Locking
SQLite connections can open two kinds of transactions, read and write transactions. This is not done explicitly, a transaction is implicitly a read-transaction until it first writes to a database table, at which point it becomes a write-transaction.

If this changed in a future version it might make more sense if the paramater naming was also inversed so bool deferred -> bool immediate

[bricelam]

@p-m-j see my previous comments on the subject:

We tried to switch to deferred transactions by default in version 2.0, but several users started seeing deadlocks in their apps (aspnet/Microsoft.Data.Sqlite#474).

[p-m-j]

@bricelam i guess someone will be upset no matter which default you choose.

How about a connection string flag to toggle the default behaviour?

[bricelam]

Hmm... File an issue so we can discuss it as a team. I'm personally against it since you'd have to be very aware of the implications for every line of SQL you write, and any time you introduce concurrency in your app. Having to set it manually every time reminds you to think about everything that might go wrong. In other words, I think the connection string keyword would harm more people than it helped.