Fix upsert for WITHOUT ROWID tables with an INTEGER primary key

GRDB/Core/Database+Schema.swift

@@ -511,11 +511,12 @@ extension Database {
             // > DESC" clause, it does not become an alias for the rowid [...]
             //
             // FIXME: We ignore the exception, and consider all INTEGER primary
-            // keys as aliases for the rowid:
-            if pkColumn.type.uppercased() == "INTEGER" {
+            // keys of rowid tables as aliases for the rowid:
+            let tableHasRowID = try fetchTableHasRowID(table)
+            if pkColumn.type.uppercased() == "INTEGER" && tableHasRowID {
                 return .rowID(pkColumn)
             } else {
-                return try .regular([pkColumn], tableHasRowID: fetchTableHasRowID(table))
+                return .regular([pkColumn], tableHasRowID: tableHasRowID)
             }
 
         default:
@@ -1605,7 +1606,19 @@ extension ForeignKeyViolation: CustomStringConvertible {
 /// pk.columns     // ["citizenID", "countryIsoCode"]
 /// pk.rowIDColumn // nil
 /// pk.isRowID     // false
+///
+/// // CREATE TABLE passport (
+/// //   id INTEGER PRIMARY KEY,
+/// //   name TEXT
+/// // ) WITHOUT ROWID
+/// let pk = try db.primaryKey("passport")
+/// pk.columns     // ["id"]
+/// pk.rowIDColumn // nil
+/// pk.isRowID     // false
 /// ```
+///
+/// An `INTEGER PRIMARY KEY` is an alias for the rowid only in rowid
+/// tables. In a `WITHOUT ROWID` table, it is a regular primary key.
 public struct PrimaryKeyInfo: Sendable {
     private enum Impl {
         /// The hidden rowID.
@@ -1659,7 +1672,9 @@ public struct PrimaryKeyInfo: Sendable {
     }
 
     /// When not nil, the name of the column that contains the
-    /// `INTEGER PRIMARY KEY`.
+    /// `INTEGER PRIMARY KEY` of a rowid table.
+    ///
+    /// This is nil for `WITHOUT ROWID` tables, which have no rowid.
     public var rowIDColumn: String? {
         switch impl {
         case .hiddenRowID:

Tests/GRDBTests/Core/PrimaryKeyInfoTests.swift

@@ -105,6 +105,36 @@ class PrimaryKeyInfoTests: GRDBTestCase {
         }
     }
 
+    func testIntegerPrimaryKeyWithoutRowID() throws {
+        let dbQueue = try makeDatabaseQueue()
+        try dbQueue.inDatabase { db in
+            try db.execute(sql: "CREATE TABLE items (id INTEGER PRIMARY KEY, name TEXT) WITHOUT ROWID")
+            let primaryKey = try db.primaryKey("items")
+            XCTAssertEqual(primaryKey.columnInfos?.map(\.name), ["id"])
+            XCTAssertEqual(primaryKey.columnInfos?.map(\.type), ["INTEGER"])
+            XCTAssertEqual(primaryKey.columns, ["id"])
+            // An INTEGER primary key is an alias for the rowid in rowid
+            // tables only: https://www.sqlite.org/lang_createtable.html
+            XCTAssertNil(primaryKey.rowIDColumn)
+            XCTAssertFalse(primaryKey.isRowID)
+            XCTAssertFalse(primaryKey.tableHasRowID)
+        }
+    }
+
+    func testIntegerPrimaryKeyWithoutRowID2() throws {
+        let dbQueue = try makeDatabaseQueue()
+        try dbQueue.inDatabase { db in
+            try db.execute(sql: "CREATE TABLE items (id INTEGER, name TEXT, PRIMARY KEY (id)) WITHOUT ROWID")
+            let primaryKey = try db.primaryKey("items")
+            XCTAssertEqual(primaryKey.columnInfos?.map(\.name), ["id"])
+            XCTAssertEqual(primaryKey.columnInfos?.map(\.type), ["INTEGER"])
+            XCTAssertEqual(primaryKey.columns, ["id"])
+            XCTAssertNil(primaryKey.rowIDColumn)
+            XCTAssertFalse(primaryKey.isRowID)
+            XCTAssertFalse(primaryKey.tableHasRowID)
+        }
+    }
+
     func testNonRowIDPrimaryKeyWithoutRowID() throws {
         let dbQueue = try makeDatabaseQueue()
         try dbQueue.inDatabase { db in

Tests/GRDBTests/Record/PersistableRecord/MutablePersistableRecordTests.swift

@@ -2388,6 +2388,81 @@ extension MutablePersistableRecordTests {
         }
     }
 
+    func test_upsert_INTEGER_primary_key_WITHOUT_ROWID() throws {
+#if GRDBCUSTOMSQLITE || SQLITE_HAS_CODEC
+        guard Database.sqliteLibVersionNumber >= 3035000 else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#else
+        guard #available(iOS 15, macOS 12, tvOS 15, watchOS 8, *) else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#endif
+
+        struct MyRecord: Codable, FetchableRecord, MutablePersistableRecord {
+            var id: Int64
+            var name: String
+        }
+
+        try makeDatabaseQueue().write { db in
+            try db.execute(sql: """
+                CREATE TABLE myRecord(
+                  id INTEGER PRIMARY KEY,
+                  name TEXT NOT NULL
+                ) WITHOUT ROWID;
+                """)
+
+            var record = MyRecord(id: 1, name: "foo")
+            try record.upsert(db)
+
+            XCTAssertEqual(lastSQLQuery, """
+                INSERT INTO "myRecord" ("id", "name") \
+                VALUES (1,'foo') \
+                ON CONFLICT DO UPDATE SET "name" = "excluded"."name"
+                """)
+        }
+    }
+
+    func test_upsertAndFetch_INTEGER_primary_key_WITHOUT_ROWID() throws {
+#if GRDBCUSTOMSQLITE || SQLITE_HAS_CODEC
+        guard Database.sqliteLibVersionNumber >= 3035000 else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#else
+        guard #available(iOS 15, macOS 12, tvOS 15, watchOS 8, *) else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#endif
+
+        struct MyRecord: Codable, FetchableRecord, MutablePersistableRecord {
+            var id: Int64
+            var name: String
+        }
+
+        try makeDatabaseQueue().write { db in
+            try db.execute(sql: """
+                CREATE TABLE myRecord(
+                  id INTEGER PRIMARY KEY,
+                  name TEXT NOT NULL
+                ) WITHOUT ROWID;
+                """)
+
+            var record = MyRecord(id: 1, name: "foo")
+            let upserted = try record.upsertAndFetch(db)
+
+            // No rowid in the RETURNING clause: the table has none
+            XCTAssertEqual(lastSQLQuery, """
+                INSERT INTO "myRecord" ("id", "name") \
+                VALUES (1,'foo') \
+                ON CONFLICT DO UPDATE SET "name" = "excluded"."name" \
+                RETURNING *
+                """)
+
+            XCTAssertEqual(upserted.id, 1)
+            XCTAssertEqual(upserted.name, "foo")
+        }
+    }
+
     func test_upsertAndFetch_do_update_set_where_with_default_strategy() throws {
 #if GRDBCUSTOMSQLITE || SQLITE_HAS_CODEC
         guard Database.sqliteLibVersionNumber >= 3035000 else {

Tests/GRDBTests/Record/PersistableRecord/PersistableRecordTests.swift

@@ -2378,6 +2378,81 @@ extension PersistableRecordTests {
         }
     }
 
+    func test_upsert_INTEGER_primary_key_WITHOUT_ROWID() throws {
+#if GRDBCUSTOMSQLITE || SQLITE_HAS_CODEC
+        guard Database.sqliteLibVersionNumber >= 3035000 else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#else
+        guard #available(iOS 15, macOS 12, tvOS 15, watchOS 8, *) else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#endif
+
+        struct MyRecord: Codable, FetchableRecord, PersistableRecord {
+            var id: Int64
+            var name: String
+        }
+
+        try makeDatabaseQueue().write { db in
+            try db.execute(sql: """
+                CREATE TABLE myRecord(
+                  id INTEGER PRIMARY KEY,
+                  name TEXT NOT NULL
+                ) WITHOUT ROWID;
+                """)
+
+            let record = MyRecord(id: 1, name: "foo")
+            try record.upsert(db)
+
+            XCTAssertEqual(lastSQLQuery, """
+                INSERT INTO "myRecord" ("id", "name") \
+                VALUES (1,'foo') \
+                ON CONFLICT DO UPDATE SET "name" = "excluded"."name"
+                """)
+        }
+    }
+
+    func test_upsertAndFetch_INTEGER_primary_key_WITHOUT_ROWID() throws {
+#if GRDBCUSTOMSQLITE || SQLITE_HAS_CODEC
+        guard Database.sqliteLibVersionNumber >= 3035000 else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#else
+        guard #available(iOS 15, macOS 12, tvOS 15, watchOS 8, *) else {
+            throw XCTSkip("UPSERT is not available")
+        }
+#endif
+
+        struct MyRecord: Codable, FetchableRecord, PersistableRecord {
+            var id: Int64
+            var name: String
+        }
+
+        try makeDatabaseQueue().write { db in
+            try db.execute(sql: """
+                CREATE TABLE myRecord(
+                  id INTEGER PRIMARY KEY,
+                  name TEXT NOT NULL
+                ) WITHOUT ROWID;
+                """)
+
+            let record = MyRecord(id: 1, name: "foo")
+            let upserted = try record.upsertAndFetch(db)
+
+            // No rowid in the RETURNING clause: the table has none
+            XCTAssertEqual(lastSQLQuery, """
+                INSERT INTO "myRecord" ("id", "name") \
+                VALUES (1,'foo') \
+                ON CONFLICT DO UPDATE SET "name" = "excluded"."name" \
+                RETURNING *
+                """)
+
+            XCTAssertEqual(upserted.id, 1)
+            XCTAssertEqual(upserted.name, "foo")
+        }
+    }
+
     func test_upsertAndFetch_do_update_set_where_with_default_strategy() throws {
 #if GRDBCUSTOMSQLITE || SQLITE_HAS_CODEC
         guard Database.sqliteLibVersionNumber >= 3035000 else {