Avoid overwriting local db column values in cases were CKRecord has no value.

Sources/SQLiteData/CloudKit/SyncEngine.swift

@@ -2009,6 +2009,12 @@
       else {
         return ""
       }
+
+      // On conflict only update columns that have a value the server record.
+      let columnNamesForUpdate = nonPrimaryKeyChangedColumns.filter { columnName in
+        record[columnName] is CKAsset || record.encryptedValues.allKeys().contains(columnName)
+      }
+
       var record = record
       let recordHasAsset = nonPrimaryKeyChangedColumns.contains { columnName in
         record[columnName] is CKAsset
@@ -2035,26 +2041,33 @@
           }
           .joined(separator: ", ")
       )
-      query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO UPDATE SET ")
-      query.append(" ")
-      query.append(
-        nonPrimaryKeyChangedColumns
-          .map { columnName in
-            if let asset = record[columnName] as? CKAsset {
-              let data = try? asset.fileURL.map { try dataManager.wrappedValue.load($0) }
-              if data == nil {
-                reportIssue("Asset data not found on disk")
+
+      // if there are no columns
+      if columnNamesForUpdate.isEmpty {
+          query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO NOTHING ")
+      } else {
+          query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO UPDATE SET")
+          query.append(" ")
+          query.append(
+            columnNamesForUpdate
+              .map { columnName in
+                if let asset = record[columnName] as? CKAsset {
+                  let data = try? asset.fileURL.map { try dataManager.wrappedValue.load($0) }
+                  if data == nil {
+                    reportIssue("Asset data not found on disk")
+                  }
+                  return "\(quote: columnName) = \(data?.queryFragment ?? #""excluded".\#(quote: columnName)"#)"
+                } else {
+                  return """
+                    \(quote: columnName) = \
+                    \(record.encryptedValues[columnName]?.queryFragment ?? #""excluded".\#(quote: columnName)"#)
+                    """
+                }
               }
-              return "\(quote: columnName) = \(data?.queryFragment ?? #""excluded".\#(quote: columnName)"#)"
-            } else {
-              return """
-                \(quote: columnName) = \
-                \(record.encryptedValues[columnName]?.queryFragment ?? #""excluded".\#(quote: columnName)"#)
-                """
-            }
-          }
-          .joined(separator: ",")
-      )
+              .joined(separator: ", ")
+          )
+      }
+
       return query
     }
   }
@@ -2437,6 +2450,14 @@
   ) -> QueryFragment {
     let allColumnNames = T.TableColumns.writableColumns.map(\.name)
     let hasNonPrimaryKeyColumns = columnNames.contains { $0 != T.primaryKey.name }
+
+    let columnNamesForUpdate = columnNames.filter { columnName in
+      if columnName == T.primaryKey.name {
+        return false
+      }
+      return (record[columnName] is CKAsset) || record.encryptedValues.allKeys().contains(columnName)
+    }
+
     var query: QueryFragment = "INSERT INTO \(T.self) ("
     query.append(allColumnNames.map { "\(quote: $0)" }.joined(separator: ", "))
     query.append(") VALUES (")
@@ -2454,11 +2475,10 @@
         .joined(separator: ", ")
     )
     query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO")
-    if hasNonPrimaryKeyColumns {
+    if !columnNamesForUpdate.isEmpty {
       query.append(" UPDATE SET ")
       query.append(
-        columnNames
-          .filter { $0 != T.primaryKey.name }
+        columnNamesForUpdate
           .map {
             """
             \(quote: $0) = "excluded".\(quote: $0)

Tests/SQLiteDataTests/Internal/Schema.swift

@@ -77,7 +77,8 @@ import SQLiteData
 @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
 func database(
   containerIdentifier: String,
-  attachMetadatabase: Bool
+  attachMetadatabase: Bool,
+  url databaseURL: URL? = nil
 ) throws -> DatabasePool {
   var configuration = Configuration()
   configuration.prepareDatabase { db in
@@ -88,8 +89,11 @@ func database(
     //   print($0.expandedDescription)
     // }
   }
-  let url = URL.temporaryDirectory.appending(path: "\(UUID().uuidString).sqlite")
+  let url = databaseURL ?? URL.temporaryDirectory.appending(path: "\(UUID().uuidString).sqlite")
   let database = try DatabasePool(path: url.path(), configuration: configuration)
+  guard databaseURL == nil else {
+    return database
+  }
   try database.write { db in
     try #sql(
       """

Tests/SQLiteDataTests/MigrationTests.swift

@@ -33,7 +33,176 @@ import Testing
   }
 }
 
+
 @available(iOS 15, *)
 @Table private struct Model {
   var date: Date
 }
+
+
+#if canImport(CloudKit)
+  @available(iOS 15, *)
+  @Table private struct User: Identifiable {
+    var id: UUID
+    var name: String
+  }
+
+  @Table("users") private struct UpdatedUser: Identifiable {
+    var id: UUID
+    var name: String
+    var honorific: String?
+  }
+
+  import CloudKit
+  import ConcurrencyExtras
+  import CustomDump
+  import InlineSnapshotTesting
+  import OrderedCollections
+  import SQLiteData
+  import SQLiteDataTestSupport
+  import SnapshotTestingCustomDump
+  import Testing
+
+  @MainActor
+  @Suite
+  final class MigrationSyncEngineTests {
+    @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
+
+    private let _container: any Sendable
+
+    var container: MockCloudContainer {
+      _container as! MockCloudContainer
+    }
+
+    let testContainerIdentifier: String
+    let databaseURL: URL
+
+    func userDatabase() throws -> UserDatabase {
+      UserDatabase(
+        database: try SQLiteDataTests.database(
+          containerIdentifier: testContainerIdentifier,
+          attachMetadatabase: false,
+          url: databaseURL
+        )
+      )
+    }
+
+
+    init() async throws {
+      testContainerIdentifier = "iCloud.co.pointfree.Testing.\(UUID())"
+      databaseURL = URL.temporaryDirectory.appending(path: "\(UUID().uuidString).sqlite")
+
+      let privateDatabase = MockCloudDatabase(databaseScope: .private)
+      let sharedDatabase = MockCloudDatabase(databaseScope: .shared)
+      let container = MockCloudContainer(
+        accountStatus: _AccountStatusScope.accountStatus,
+        containerIdentifier: testContainerIdentifier,
+        privateCloudDatabase: privateDatabase,
+        sharedCloudDatabase: sharedDatabase
+      )
+      _container = container
+      privateDatabase.set(container: container)
+      sharedDatabase.set(container: container)
+    }
+
+    @available(iOS 15, *)
+    @Test func handleDataMigration() async throws {
+      let userDatabase = try userDatabase()
+      // Do first migration
+      try await userDatabase.userWrite { db in
+        try #sql(
+          """
+          CREATE TABLE "users" (
+            "id" TEXT PRIMARY KEY NOT NULL ON CONFLICT REPLACE DEFAULT (uuid()),
+            "name" TEXT NOT NULL
+          )
+          """
+        )
+        .execute(db)
+      }
+
+      var syncEngine: Optional<SyncEngine> = try await SyncEngine(
+        container: container,
+        userDatabase: userDatabase,
+        delegate: nil,
+        privateTables: User.self,
+        startImmediately: true
+      )
+
+      let currentUserRecordID = CKRecord.ID(
+        recordName: "currentUser"
+      )
+
+      await syncEngine!.handleEvent(
+        .accountChange(changeType: .signIn(currentUser: currentUserRecordID)),
+        syncEngine: syncEngine!.private
+      )
+      await syncEngine!.handleEvent(
+        .accountChange(changeType: .signIn(currentUser: currentUserRecordID)),
+        syncEngine: syncEngine!.shared
+      )
+      try await syncEngine!.processPendingDatabaseChanges(scope: .private)
+
+      try await userDatabase.userWrite { db in
+        try User.insert {
+          User.Draft(name: "Bob")
+          User.Draft(name: "Alice")
+          User.Draft(name: "Alfred")
+        }.execute(db)
+      }
+
+      // Do we need to await something here?
+      try await Task.sleep(for: .seconds(1))
+      try await syncEngine?.processPendingRecordZoneChanges(scope: .private)
+      syncEngine?.stop()
+      syncEngine = nil
+
+      try userDatabase.database.close()
+
+      let newDbConnection = try self.userDatabase()
+
+      try await newDbConnection.userWrite { db in
+        try #sql(
+          """
+          ALTER TABLE "users"
+          ADD COLUMN "honorific" TEXT
+          """
+        )
+        .execute(db)
+
+        let existingUsers = try UpdatedUser.all.fetchAll(db)
+        for user in existingUsers {
+          switch user.name.lowercased() {
+          case "bob":
+            try UpdatedUser.find(user.id).update {
+              $0.honorific = "Mr"
+            }.execute(db)
+          case "alice":
+            try UpdatedUser.find(user.id).update {
+              $0.honorific = "Ms"
+            }.execute(db)
+          default:
+            continue
+          }
+        }
+      }
+
+      syncEngine = try await SyncEngine(
+        container: container,
+        userDatabase: newDbConnection,
+        delegate: nil,
+        privateTables: UpdatedUser.self,
+        startImmediately: true
+      )
+
+      let bob = try await newDbConnection.read { db in
+        try UpdatedUser.all.where { $0.name.eq("Bob") }.fetchOne(db)
+      }
+      #expect(bob?.honorific == "Mr")
+    }
+
+
+
+    // Do we need to wait here...
+  }
+#endif