Merge pull request #1602 from groue/dev/GRDB7-remove-default-transact…

…ion-kind

GRDB 7: Perform all writes with immediate transactions by default

GRDB/Core/Configuration.swift

@@ -238,28 +238,6 @@ public struct Configuration {
 
     // MARK: - Transactions
 
-    /// The default kind of write transactions.
-    ///
-    /// The default is ``Database/TransactionKind/deferred``.
-    ///
-    /// You can change the default transaction kind. For example, you can force
-    /// all write transactions to be `IMMEDIATE`:
-    ///
-    /// ```swift
-    /// var config = Configuration()
-    /// config.defaultTransactionKind = .immediate
-    /// let dbQueue = try DatabaseQueue(configuration: config)
-    ///
-    /// // BEGIN IMMEDIATE TRANSACTION; ...; COMMIT TRANSACTION;
-    /// try dbQueue.write { db in ... }
-    /// ```
-    ///
-    /// This property is ignored for read-only transactions. Those always open
-    /// `DEFERRED` SQLite transactions.
-    ///
-    /// Related SQLite documentation: <https://www.sqlite.org/lang_transaction.html>
-    public var defaultTransactionKind: Database.TransactionKind = .deferred
-
     /// A boolean value indicating whether it is valid to leave a transaction
     /// opened at the end of a database access method.
     ///

GRDB/Core/Database.swift

@@ -1282,14 +1282,10 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
     ///   Use ``inSavepoint(_:)`` instead.
     ///
     /// - parameters:
-    ///     - kind: The transaction type (default nil).
+    ///     - kind: The transaction type.
     ///
-    ///       If nil, and the database connection is read-only, the transaction
-    ///       kind is ``TransactionKind/deferred``.
-    ///
-    ///       If nil, and the database connection is not read-only, the
-    ///       transaction kind is the ``Configuration/defaultTransactionKind``
-    ///       of the ``configuration``.
+    ///       If nil, the transaction kind is DEFERRED when the current
+    ///       database access is read-only, and IMMEDIATE otherwise.
     ///     - operations: A function that executes SQL statements and returns
     ///       either ``TransactionCompletion/commit`` or ``TransactionCompletion/rollback``.
     /// - throws: A ``DatabaseError`` whenever an SQLite error occurs, or the
@@ -1413,8 +1409,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
             // By default, top level SQLite savepoints open a
             // deferred transaction.
             //
-            // But GRDB database configuration mandates a default transaction
-            // kind that we have to honor.
+            // But GRDB prefers immediate transactions for writes.
             //
             // Besides, starting some (?) SQLCipher/SQLite version, SQLite has a
             // bug. Returning 1 from `sqlite3_commit_hook` does not leave the
@@ -1502,18 +1497,22 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
     /// Related SQLite documentation: <https://www.sqlite.org/lang_transaction.html>
     ///
     /// - parameters:
-    ///     - kind: The transaction type (default nil).
-    ///
-    ///       If nil, and the database connection is read-only, the transaction
-    ///       kind is ``TransactionKind/deferred``.
+    ///     - kind: The transaction type.
     ///
-    ///       If nil, and the database connection is not read-only, the
-    ///       transaction kind is the ``Configuration/defaultTransactionKind``
-    ///       of the ``configuration``.
+    ///       If nil, the transaction kind is DEFERRED when the current
+    ///       database access is read-only, and IMMEDIATE otherwise.
     /// - throws: A ``DatabaseError`` whenever an SQLite error occurs.
     public func beginTransaction(_ kind: TransactionKind? = nil) throws {
         // SQLite throws an error for non-deferred transactions when read-only.
-        let kind = kind ?? (isReadOnly ? .deferred : configuration.defaultTransactionKind)
+        // We prefer immediate transactions for writes, so that write
+        // transactions can not overlap. This reduces the opportunity for
+        // SQLITE_BUSY, which is immediately thrown whenever a transaction
+        // is upgraded after an initial read and a concurrent processes
+        // has acquired the write lock beforehand. This SQLITE_BUSY error
+        // can not be avoided with a busy timeout.
+        //
+        // See <https://github.com/groue/GRDB.swift/issues/1483>.
+        let kind = kind ?? (isReadOnly ? .deferred : .immediate)
         try execute(sql: "BEGIN \(kind.rawValue) TRANSACTION")
         assert(sqlite3_get_autocommit(sqliteConnection) == 0)
     }

GRDB/Core/DatabasePool.swift

@@ -118,10 +118,6 @@ public final class DatabasePool {
 
         configuration.readonly = true
 
-        // Readers use deferred transactions by default.
-        // Other transaction kinds are forbidden by SQLite in read-only connections.
-        configuration.defaultTransactionKind = .deferred
-
         // <https://www.sqlite.org/wal.html#sometimes_queries_return_sqlite_busy_in_wal_mode>
         // > But there are some obscure cases where a query against a WAL-mode
         // > database can return SQLITE_BUSY, so applications should be prepared
@@ -787,9 +783,10 @@ extension DatabasePool: DatabaseWriter {
     ///
     /// - precondition: This method is not reentrant.
     /// - parameters:
-    ///     - kind: The transaction type (default nil). If nil, the transaction
-    ///       type is the ``Configuration/defaultTransactionKind`` of the
-    ///       the ``configuration``.
+    ///     - kind: The transaction type.
+    ///
+    ///       If nil, the transaction kind is DEFERRED when the database
+    ///       connection is read-only, and IMMEDIATE otherwise.
     ///     - updates: A function that updates the database.
     /// - throws: The error thrown by `updates`, or by the wrapping transaction.
     public func writeInTransaction(

GRDB/Core/DatabaseQueue.swift

@@ -352,9 +352,10 @@ extension DatabaseQueue: DatabaseWriter {
     /// ```
     ///
     /// - parameters:
-    ///     - kind: The transaction type (default nil). If nil, the transaction
-    ///       type is the ``Configuration/defaultTransactionKind`` of the
-    ///       the ``configuration``.
+    ///     - kind: The transaction type.
+    ///
+    ///       If nil, the transaction kind is DEFERRED when the database
+    ///       connection is read-only, and IMMEDIATE otherwise.
     ///     - updates: A function that updates the database.
     /// - throws: The error thrown by `updates`, or by the wrapping transaction.
     public func inTransaction(

GRDB/Core/DatabaseSnapshot.swift

@@ -126,10 +126,6 @@ public final class DatabaseSnapshot {
         // DatabaseSnapshot is read-only.
         configuration.readonly = true
 
-        // DatabaseSnapshot uses deferred transactions by default.
-        // Other transaction kinds are forbidden by SQLite in read-only connections.
-        configuration.defaultTransactionKind = .deferred
-
         // DatabaseSnapshot keeps a long-lived transaction.
         configuration.allowsUnsafeTransactions = true
 

GRDB/Core/DatabaseSnapshotPool.swift

@@ -242,10 +242,6 @@ public final class DatabaseSnapshotPool {
         // DatabaseSnapshotPool is read-only.
         configuration.readonly = true
 
-        // DatabaseSnapshotPool uses deferred transactions by default.
-        // Other transaction kinds are forbidden by SQLite in read-only connections.
-        configuration.defaultTransactionKind = .deferred
-
         // DatabaseSnapshotPool keeps a long-lived transaction.
         configuration.allowsUnsafeTransactions = true
 

GRDB/Documentation.docc/DatabaseSharing.md

@@ -152,12 +152,11 @@ If several processes want to write in the database, configure the database pool
 
 ```swift
 var configuration = Configuration()
-configuration.defaultTransactionKind = .immediate
 configuration.busyMode = .timeout(/* a TimeInterval */)
 let dbPool = try DatabasePool(path: ..., configuration: configuration)
 ```
 
-Both the `defaultTransactionKind` and `busyMode` are important for preventing `SQLITE_BUSY`. The `immediate` transaction kind prevents write transactions from overlapping, and the busy timeout has write transactions wait, instead of throwing `SQLITE_BUSY`, whenever another process is writing.
+The busy timeout has write transactions wait, instead of throwing `SQLITE_BUSY`, whenever another process is writing. GRDB automatically opens all write transactions with the IMMEDIATE kind, preventing write transactions from overlapping.
 
 With such a setup, you will still get `SQLITE_BUSY` errors if the database remains locked by another process for longer than the specified timeout. You can catch those errors:
 

GRDB/Documentation.docc/Extension/Configuration.md

@@ -90,7 +90,6 @@ do {
 ### Configuring GRDB Connections
 
 - ``allowsUnsafeTransactions``
-- ``defaultTransactionKind``
 - ``label``
 - ``maximumReaderCount``
 - ``observesSuspensionNotifications``

GRDB/Documentation.docc/Transactions.md

@@ -214,6 +214,8 @@ SQLite savepoints are more than nested transactions, though. For advanced uses,
 
 SQLite supports [three kinds of transactions](https://www.sqlite.org/lang_transaction.html): deferred (the default), immediate, and exclusive.
 
+By default, GRDB opens DEFERRED transaction for reads, and IMMEDIATE transactions for writes.
+
 The transaction kind can be chosen for individual transaction:
 
 ```swift
@@ -222,20 +224,3 @@ let dbQueue = try DatabaseQueue(path: "/path/to/database.sqlite")
 // BEGIN EXCLUSIVE TRANSACTION ...
 try dbQueue.inTransaction(.exclusive) { db in ... }
 ```
-
-It is also possible to configure the ``Configuration/defaultTransactionKind``:
-
-```swift
-var config = Configuration()
-config.defaultTransactionKind = .immediate
-
-let dbQueue = try DatabaseQueue(
-    path: "/path/to/database.sqlite",
-    configuration: config)
-
-// BEGIN IMMEDIATE TRANSACTION ...
-try dbQueue.write { db in ... }
-
-// BEGIN IMMEDIATE TRANSACTION ...
-try dbQueue.inTransaction { db in ... }
-```

TODO.md

@@ -90,7 +90,7 @@
 - [X] GRDB7/BREAKING: insertAndFetch, saveAndFetch, and updateAndFetch no longer return optionals (32f41472)
 - [ ] GRDB7/BREAKING: AsyncValueObservation does not need any scheduler (83c0e643)
 - [X] GRDB7/BREAKING: Stop exporting SQLite (679d6463)
-- [ ] GRDB7/BREAKING: Remove Configuration.defaultTransactionKind (2661ff46)
+- [X] GRDB7/BREAKING: Remove Configuration.defaultTransactionKind (2661ff46)
 - [ ] GRDB7: Replace LockedBox with Mutex (00ccab06)
 - [ ] GRDB7: Sendable: BusyCallback (e0d8e20b)
 - [ ] GRDB7: Sendable: BusyMode (e0d8e20b)

Tests/GRDBTests/BackupTestCase.swift

@@ -84,7 +84,7 @@ class BackupTestCase: GRDBTestCase {
         let sourceDbPageCount = try setupBackupSource(source)
         try setupBackupDestination(destination)
 
-        try source.write { sourceDb in
+        try source.read { sourceDb in
             try destination.barrierWriteWithoutTransaction { destDb in
                 XCTAssertThrowsError(
                     try sourceDb.backup(to: destDb, pagesPerStep: 1) { progress in
@@ -102,7 +102,7 @@ class BackupTestCase: GRDBTestCase {
             XCTAssertEqual(try Int.fetchOne(db, sql: "SELECT id FROM items")!, 1)
         }
 
-        try source.write { dbSource in
+        try source.read { dbSource in
             try destination.barrierWriteWithoutTransaction { dbDest in
                 var progressCount: Int = 1
                 var isCompleted: Bool = false

Tests/GRDBTests/DatabaseQueueTests.swift

@@ -369,8 +369,6 @@ class DatabaseQueueTests: GRDBTestCase {
     func test_busy_timeout_and_IMMEDIATE_transactions_do_prevent_SQLITE_BUSY() throws {
         var configuration = dbConfiguration!
         // Test fails when this line is commented
-        configuration.defaultTransactionKind = .immediate
-        // Test fails when this line is commented
         configuration.busyMode = .timeout(10)
 
         let dbQueue = try makeDatabaseQueue(filename: "test")