Merge pull request #1822 from groue/dev/issue-1820

Fix the save() method for record types with attached triggers

Author: groue

GRDB/Core/Database.swift

@@ -2018,6 +2018,34 @@ extension Database {
 }
 #endif
 
+extension Database {
+    /// Returns the count of changes executed by one statement execution.
+    func countChanges<T>(_ count: inout Int, forTable tableName: String, updates: () throws -> T) throws -> T {
+        // Database.changesCount calls sqlite3_changes(), whose documentation says:
+        //
+        // > https://sqlite.org/c3ref/changes.html
+        // > Changes to a view that are intercepted by INSTEAD OF triggers are not counted.
+        //
+        // We want to support INSTEAD OF triggers, so we prefer to use
+        // sqlite3_total_changes() for views.
+        //
+        // At the same time, FTS5 has sqlite3_total_changes() report changes
+        // even when database is not changed (https://github.com/groue/GRDB.swift/issues/1820)
+        //
+        // Well, let's do our best:
+        if try viewExists(tableName) {
+            let prevCount = totalChangesCount
+            let result = try updates()
+            count = totalChangesCount - prevCount
+            return result
+        } else {
+            let result = try updates()
+            count = changesCount
+            return result
+        }
+    }
+}
+
 extension Database {
 
     // MARK: - Database-Related Types

GRDB/QueryInterface/Request/QueryInterfaceRequest.swift

@@ -635,9 +635,11 @@ extension QueryInterfaceRequest {
     @discardableResult
     public func deleteAll(_ db: Database) throws -> Int {
         let statement = try SQLQueryGenerator(relation: relation).makeDeleteStatement(db)
-        let prevCount = db.totalChangesCount
-        try statement.execute()
-        return db.totalChangesCount - prevCount
+        var changesCount = 0
+        try db.countChanges(&changesCount, forTable: relation.source.tableName) {
+            try statement.execute()
+        }
+        return changesCount
     }
 }
 
@@ -1136,9 +1138,11 @@ extension QueryInterfaceRequest {
             // database not hit
             return 0
         }
-        let prevCount = db.totalChangesCount
-        try updateStatement.execute()
-        return db.totalChangesCount - prevCount
+        var changesCount = 0
+        try db.countChanges(&changesCount, forTable: relation.source.tableName) {
+            try updateStatement.execute()
+        }
+        return changesCount
     }
 
     /// Updates matching rows, and returns the number of updated rows.

GRDB/Record/MutablePersistableRecord+Delete.swift

@@ -53,8 +53,10 @@ extension MutablePersistableRecord {
             // Nil primary key
             return false
         }
-        let prevCount = db.totalChangesCount
-        try statement.execute()
-        return (db.totalChangesCount - prevCount) > 0
+        var changesCount = 0
+        try db.countChanges(&changesCount, forTable: type(of: self).databaseTableName) {
+            try statement.execute()
+        }
+        return changesCount > 0
     }
 }

GRDB/Record/MutablePersistableRecord+Update.swift

@@ -956,9 +956,10 @@ extension MutablePersistableRecord {
             // Nil primary key
             try dao.recordNotFound()
         }
-        let prevCount = db.totalChangesCount
-        let returned = try fetch(statement)
-        let changesCount = db.totalChangesCount - prevCount
+        var changesCount = 0
+        let returned = try db.countChanges(&changesCount, forTable: type(of: self).databaseTableName) {
+            try fetch(statement)
+        }
         if changesCount == 0 {
             // No row was updated
             try dao.recordNotFound()

Tests/GRDBTests/MutablePersistableRecordTests.swift

@@ -2796,3 +2796,47 @@ extension MutablePersistableRecordTests {
         } catch DatabaseError.SQLITE_MISUSE { }
     }
 }
+
+#if SQLITE_ENABLE_FTS5
+class Issue1820Tests: GRDBTestCase {
+    // Regression test for https://github.com/groue/GRDB.swift/issues/1820
+    func testIssue1820() throws {
+        struct Serving: Codable, FetchableRecord, PersistableRecord {
+            let id: UUID
+            var description: String
+            var foodId: String
+            
+            static let author = hasOne(Food.self)
+        }
+        
+        struct Food: Codable, FetchableRecord, PersistableRecord {
+            let id: UUID
+            var name: String
+            var foodId: String
+        }
+        
+        let dbQueue = try makeDatabaseQueue()
+        try dbQueue.write { db in
+            try db.create(table: "food") { t in
+                t.column("id", .blob).primaryKey()
+                t.column("name", .text)
+                t.column("foodId", .text).unique()
+            }
+            
+            try db.create(table: "serving") { t in
+                t.column("id", .blob).primaryKey()
+                t.column("description", .text)
+                t.column("foodId", .text).references("food", column: "foodId")
+            }
+            
+            try db.create(virtualTable: "food_fts", using: FTS5()) { t in
+                t.synchronize(withTable: "food")
+                t.column("name")
+            }
+            
+            try Food(id: UUID(), name: "Apple", foodId: "apple").save(db)
+            try Serving(id: UUID(), description: "Apple", foodId: "apple").save(db)
+        }
+    }
+}
+#endif