Skip to content

Fix 'NULL DEFAULT _' migrations. #380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mbrandonw wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix 'NULL DEFAULT _' migrations. #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
37e20be
Fix 'NULL DEFAULT _' migrations.
mbrandonw Jan 20, 2026
ca96103
clean up
mbrandonw Jan 21, 2026
eebb27b
Add test for when old device makes change and syncs to new device.
mbrandonw Jan 21, 2026
f9a517f
fixes
mbrandonw Jan 21, 2026
9b978a8
clean up
mbrandonw Jan 21, 2026
3d79965
filter columnNames by allColumnNames
mbrandonw Jan 22, 2026
c488f99
clean up
mbrandonw Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@

@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
extension CKRecord {
func hasSet(key: String) -> Bool {
self.encryptedValues["\(CKRecord.userModificationTimeKey)_\(key)"] != nil
}

@discardableResult
package func setValue(
_ newValue: some CKRecordValueProtocol & Equatable,
Expand Down Expand Up @@ -233,6 +237,9 @@
encryptedValues[at: key] = userModificationTime
self.userModificationTime = userModificationTime
return true
} else if !hasSet(key: key) {
encryptedValues[at: key] = userModificationTime
self.userModificationTime = userModificationTime
}
return false
}
Expand Down
16 changes: 12 additions & 4 deletions Sources/SQLiteData/CloudKit/SyncEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2003,7 +2003,9 @@
) async throws -> QueryFragment {
let nonPrimaryKeyChangedColumns =
changedColumnNames
.filter { $0 != T.primaryKey.name }
.filter {
$0 != T.primaryKey.name && record.hasSet(key: $0)
}
guard
!nonPrimaryKeyChangedColumns.isEmpty
else {
Expand Down Expand Up @@ -2435,13 +2437,19 @@
record: CKRecord,
columnNames: some Collection<String>
) -> QueryFragment {
let allColumnNames = T.TableColumns.writableColumns.map(\.name)
let setColumnNames = T.TableColumns.writableColumns.map(\.name)
.filter { record.hasSet(key: $0) }
guard !setColumnNames.isEmpty
else {
return ""
}
let columnNames = columnNames.filter { setColumnNames.contains($0) }
let hasNonPrimaryKeyColumns = columnNames.contains { $0 != T.primaryKey.name }
Copy link

@hishnash hishnash Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible that columnNames here somehow ends up with the column name that is not in the filtered allColumnNames and thus would result in a SQL error as columnNames is used in the ON CONFLICT DO SET option. Maybe columnNames need to drop any names that are not found in allColumnNames (I know there is some other method doing this further up before upsert is called but that method does not consider the hasSet(key: $0) )

Copy link
Member Author

@mbrandonw mbrandonw Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that seems theoretically possible, but haven't thought of the exact scenario where that might happen. I pushed an updated.

var query: QueryFragment = "INSERT INTO \(T.self) ("
query.append(allColumnNames.map { "\(quote: $0)" }.joined(separator: ", "))
query.append(setColumnNames.map { "\(quote: $0)" }.joined(separator: ", "))
query.append(") VALUES (")
query.append(
allColumnNames
setColumnNames
.map { columnName in
if let asset = record[columnName] as? CKAsset {
@Dependency(\.dataManager) var dataManager
Expand Down
36 changes: 18 additions & 18 deletions Tests/SQLiteDataTests/CloudKitTests/CloudKitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
tableName: "remindersLists",
schema: """
CREATE TABLE "remindersLists" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"title" TEXT NOT NULL ON CONFLICT REPLACE DEFAULT ''
) STRICT
""",
Expand All @@ -34,7 +34,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[1]: TableInfo(
defaultValue: "\'\'",
Expand Down Expand Up @@ -101,7 +101,7 @@
tableName: "reminders",
schema: """
CREATE TABLE "reminders" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"dueDate" TEXT,
"isCompleted" INTEGER NOT NULL ON CONFLICT REPLACE DEFAULT 0,
"priority" INTEGER,
Expand All @@ -124,7 +124,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[2]: TableInfo(
defaultValue: "0",
Expand Down Expand Up @@ -177,7 +177,7 @@
tableName: "reminderTags",
schema: """
CREATE TABLE "reminderTags" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"reminderID" INTEGER NOT NULL REFERENCES "reminders"("id") ON DELETE CASCADE,
"tagID" TEXT NOT NULL REFERENCES "tags"("title") ON DELETE CASCADE ON UPDATE CASCADE
) STRICT
Expand All @@ -188,7 +188,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[1]: TableInfo(
defaultValue: nil,
Expand All @@ -210,7 +210,7 @@
tableName: "parents",
schema: """
CREATE TABLE "parents"(
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL
"id" INT PRIMARY KEY NOT NULL
) STRICT
""",
tableInfo: [
Expand All @@ -219,15 +219,15 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
)
]
),
[7]: RecordType(
tableName: "childWithOnDeleteSetNulls",
schema: """
CREATE TABLE "childWithOnDeleteSetNulls"(
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"parentID" INTEGER REFERENCES "parents"("id") ON DELETE SET NULL ON UPDATE SET NULL
) STRICT
""",
Expand All @@ -237,7 +237,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[1]: TableInfo(
defaultValue: nil,
Expand All @@ -252,7 +252,7 @@
tableName: "childWithOnDeleteSetDefaults",
schema: """
CREATE TABLE "childWithOnDeleteSetDefaults"(
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"parentID" INTEGER NOT NULL DEFAULT 0
REFERENCES "parents"("id") ON DELETE SET DEFAULT ON UPDATE SET DEFAULT
) STRICT
Expand All @@ -263,7 +263,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[1]: TableInfo(
defaultValue: "0",
Expand All @@ -278,7 +278,7 @@
tableName: "modelAs",
schema: """
CREATE TABLE "modelAs" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"count" INTEGER NOT NULL ON CONFLICT REPLACE DEFAULT 0,
"isEven" INTEGER GENERATED ALWAYS AS ("count" % 2 == 0) VIRTUAL
)
Expand All @@ -296,15 +296,15 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
)
]
),
[10]: RecordType(
tableName: "modelBs",
schema: """
CREATE TABLE "modelBs" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"isOn" INTEGER NOT NULL ON CONFLICT REPLACE DEFAULT 0,
"modelAID" INTEGER NOT NULL REFERENCES "modelAs"("id") ON DELETE CASCADE
)
Expand All @@ -315,7 +315,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[1]: TableInfo(
defaultValue: "0",
Expand All @@ -337,7 +337,7 @@
tableName: "modelCs",
schema: """
CREATE TABLE "modelCs" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id" INT PRIMARY KEY NOT NULL,
"title" TEXT NOT NULL ON CONFLICT REPLACE DEFAULT '',
"modelBID" INTEGER NOT NULL REFERENCES "modelBs"("id") ON DELETE CASCADE
)
Expand All @@ -348,7 +348,7 @@
isPrimaryKey: true,
name: "id",
isNotNull: true,
type: "INTEGER"
type: "INT"
),
[1]: TableInfo(
defaultValue: nil,
Expand Down
4 changes: 2 additions & 2 deletions Tests/SQLiteDataTests/CloudKitTests/FetchRecordZoneChangesTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func receiveRecord_SingleFieldPrimaryKey() async throws {
let tagRecord = CKRecord(recordType: "tags", recordID: Tag.recordID(for: "weekend"))
tagRecord.encryptedValues["title"] = "weekend"
let tagRecord = CKRecord(recordType: Tag.tableName, recordID: Tag.recordID(for: "weekend"))
tagRecord.setValue("weekend", forKey: "title", at: 0)
try await syncEngine.modifyRecords(scope: .private, saving: [tagRecord]).notify()

try await userDatabase.read { db in
Expand Down
6 changes: 5 additions & 1 deletion Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,13 @@
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func remoteCreatesRecordABC_localReceivesAC_remoteDeletesBC() async throws {
let modelARecord = CKRecord(recordType: ModelA.tableName, recordID: ModelA.recordID(for: 1))
modelARecord.setValue(1, forKey: "id", at: now)
let modelBRecord = CKRecord(recordType: ModelB.tableName, recordID: ModelB.recordID(for: 1))
modelBRecord.setValue(1, forKey: "id", at: now)
modelBRecord.setValue(1, forKey: "modelAID", at: now)
modelBRecord.parent = CKRecord.Reference(record: modelARecord, action: .none)
let modelCRecord = CKRecord(recordType: ModelC.tableName, recordID: ModelC.recordID(for: 1))
modelCRecord.setValue(1, forKey: "id", at: now)
modelCRecord.setValue(1, forKey: "modelBID", at: now)
modelCRecord.parent = CKRecord.Reference(record: modelBRecord, action: .none)

Expand Down Expand Up @@ -205,7 +208,8 @@
recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__),
recordType: "modelAs",
parent: nil,
share: nil
share: nil,
id: 1
)
]
),
Expand Down
Loading