Skip to content
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.

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

What's the "right" way to use autoIncrementedPrimaryKey? #1435

Closed
OscarApeland opened this issue Sep 29, 2023 · 6 comments
Closed

What's the "right" way to use autoIncrementedPrimaryKey? #1435

OscarApeland opened this issue Sep 29, 2023 · 6 comments

Comments

@OscarApeland
Copy link

OscarApeland commented Sep 29, 2023

Fresh to GRDB. Loving it so far.

I can't seem to find documentation on the preferred method for creating objects with auto-incrementing IDs. Lets say I'm creating a table like this.

try db.create(table: "row") { t in
    t.autoIncrementedPrimaryKey("id")
}

First of all, should this property be Int or Int64? Then, how should the property be defined in the corresponding record?

struct Row: ... {
    let id: Int 
    // or
    let id: Int!
    // or 
    let id: Int?
}

It seems like it needs to be either optional or implicitly-unwrapped-optional, because just initing with Row(id: 0).insertAndFetch(db) crashes due to non-unique IDs. Initing the model with Row(id: nil).insertAndFetch(db) works, but that requires the Optional/IUO.

Is using IUO's the intended method for creating objects with auto-incremented IDs?

@groue
Copy link
Owner

groue commented Sep 29, 2023

Hello @OscarApeland,

I think you'll find GRBD documentation that is slightly outdated about this, so your question is a perfect opportunity to clarify ideas about SQLite auto-incremented ids. It will be easier to update the doc after that.

The recommended type for auto-incremented ids is the optional Int64?

// THE RECOMMENDED SETUP FOR AUTO-INCREMENTED IDS

// The database schema
try db.create(table: "player") { t in
    t.autoIncrementedPrimaryKey("id")
    t.column("name", .text).notNull()
}

// The record type
struct Player {
    var id: Int64?
    var name: String
}

extension Player: Codable, FetchableRecord, MutablePersistableRecord {
    // Update auto-incremented id upon successful insertion
    mutating func didInsert(_ inserted: InsertionSuccess) {
        id = inserted.rowID
    }
}

// Usage
try dbQueue.write { db in
    var player = Player(id: nil, name: "Zoé")
    try player.insert(db)
    player.id // not nil 🙂
}

Why Int64 instead of Int?

Int64 is sure to fit the 64-bit rowids generated by SQLite.

Int will often work just as well, because Int is 64 bits on most platforms and devices. But sometimes Int is only 32-bits. And since we generally do not make any computations with such ids, Int has no real advantage when compared to Int64.

Why an optional?

Because it makes it possible to use nil for records that are not saved yet, waiting for their auto-incremented id:

var player = Player(id: nil, name: "Albert")

// An unsaved player has a nil id:
player.id // nil

// A new id is generated when player is inserted:
try player.insert(db)
player.id // 42

If you used a non-optional Int64, you couldn't instantiate a Player without any id as in the above sample code, and you'd have to come up with some unique id yourself, without profiting from the SQLite auto-increment feature. Sounds like a missed target.

If you used an implicitly unwrapped optional Int64!, your application would not be able to deal with unsaved records safely:

struct Player {
    var id: Int64!
    var name: String
}

let player = Player(name: "Beatrix") // Looks innocuous, right?
print(player.id)                     // 💥 Well, it is not

This is wrong, because being unsaved is a normal state. Many records start their life unsaved until they are inserted in the database, and get an id. Unsaved records must not be dangerous.

Don't make such record types conform to Identifiable.

That's where I should update the documentation:

// BAD IDEA: Identifiable type with an optional id
struct Player: Identifiable {
    var id: Int64?
}

This makes it difficult to refer to a non-optional player id:

// This is an optional, but it does not look like it is.
// That's because `Player.ID` is `Int64?`
// It's impossible to express "a non-optional Player.ID".
// One has to write `Int64`, which is ok, but less clear.
let playerId: Player.ID

This makes two unsaved players identical as far as Identifiable is concerned, with plenty of surprising interactions with SwiftUI and other libraries:

let christopher = Player(id: nil, name: "Christopher")
let diane = Player(id: nil, name: "Diane")

// 100% normal, and very unsettling at the same time.
christopher.id == diane.id // true

The truth is that unsaved players are not identifiable, because they don't have any id yet. And that's why it is not recommended to have types with optional ids conform to Identifiable.

Even when the record type does not conform to Identifiable, you can still define an Player.ID type:

struct Player {
    typealias ID = Int64 // not optional!
    var id: ID?
}

// This time, unambiguously non-optional:
let playerId: Player.ID

Going Further: safer IDs with Tagged

The Tagged library makes it possible to create types that can't be mixed together, so that for example one can't use a Player.ID in apis that expect a Team.ID. Tagged ids are safer than raw Int64.

To use auto-incremented ids with Tagged, write:

struct Player {
    typealias ID = Tagged<Self, Int64> // not optional!
    var id: ID?
}

extension Player: FetchableRecord, MutablePersistableRecord {
    /// Updates auto-incremented id upon successful insertion
    mutating func didInsert(_ inserted: InsertionSuccess) {
        id = ID(rawValue: inserted.rowID)
    }
}

It is recommended to always define tagged ID types with an explicit typealias, as above. The alternative below is less good, because it can create compiler errors in unexpected places:

// NOT RECOMMENDED
// This compiles fine...
struct Player: Identifiable {
    var id: Tagged<Self, Int64>
}

// ... but this does not compile:
extension Player.ID {
    // ❌ Compiler error:
    // Extension of type 'Player.ID' must be declared as an
    // extension of 'Tagged<Player, Int64>'
    static let magic: Player.ID = 42
}

To complete the integration of Tagged, add somewhere in your app the few lines below. They make it possible to use Tagged values at all places where GRDB accepts plain values such as Int64 and others:

// Tagged+GRDB.swift
import GRDB
import Tagged

// Add database support to Tagged values
extension Tagged: SQLExpressible where RawValue: SQLExpressible { }
extension Tagged: StatementBinding where RawValue: StatementBinding { }
extension Tagged: StatementColumnConvertible where RawValue: StatementColumnConvertible { }
extension Tagged: DatabaseValueConvertible where RawValue: DatabaseValueConvertible { }

Going Further: distinct types for unsaved and saved records

When Identifiable conformance is a must, and you still need auto-incremented ids, you can consider using two distinct types. In the example below, the first type Player has an optional id, and does not conform to Identifiable for the reasons explained above. The second type PersistedPlayer has a non-optional id, and safely conforms to Identifiable:

/// `Player` deals with players (saved and unsaved ones)
struct Player {
    var id: Int64? // optional
    var name: String
    var score: Int
}

extension Player: Encodable, MutablePersistableRecord {
    /// Updates auto-incremented id upon successful insertion
    mutating func didInsert(_ inserted: InsertionSuccess) {
        id = inserted.rowID
    }
}

/// `PersistedPlayer` deals with saved players only
struct PersistedPlayer: Identifiable {
    let id: Int64 // not optional
    var name: String
    var score: Int
}

extension PersistedPlayer: Codable, FetchableRecord, PersistableRecord {
    static var databaseTableName: String { Player.databaseTableName }
}

Usage:

// Fetch
try dbQueue.read { db 
    let persistedPlayer = try PersistedPlayer.find(db, id: 1)
    persistedPlayer.id // not optional
}

// Insert
try dbQueue.write { db in
    var player = Player(id: nil, name: "Arthur", score: 1000)
    player.id // nil
    
    let persistedPlayer = try player.insertAndFetch(db, as: PersistedPlayer.self)!
    persistedPlayer.id // not optional
}

(Note the ! after insertAndFetch: it's safe because this method can't return nil. Precisely speaking, it could return nil if the Player type would use an IGNORE conflict policy on insert - but this obscure setup never happens by chance. A future GRDB version will probably improve this api.)

Going Further: completely avoid optionals

This can be achieved with two types, as in the above sample code. The difference is that the Player type has no id property at all. Such type has no access to some features, such as updating specific rows in the database. One always has to use PersistedPlayer for that.

/// `Player` deals with unsaved players
struct Player {
    var name: String
    var score: Int
}

extension Player: Codable, FetchableRecord, PersistableRecord { }

/// `PersistedPlayer` deals with saved players only
struct PersistedPlayer {
    var id: Int64
    var name: String
    var score: Int
}

extension PersistedPlayer: Codable, FetchableRecord, PersistableRecord {
    static var databaseTableName: String { Player.databaseTableName }
}

// Usage
try dbQueue.write { db in
    // Fetch
    let persistedPlayer = try PersistedPlayer.find(db, id: 1)
    persistedPlayer.id // not optional
    
    // Insert
    let player = Player(name: "Arthur", score: 100)
    let persistedPlayer = try player.insertAndFetch(db, as: PersistedPlayer.self)!
    persistedPlayer.id // not optional
}

Revisions

  • 2023/10/20: Added the Tagged extensions that complete the integration with GRDB.

@OscarApeland
Copy link
Author

That clarified everything I was wondering about and more - thank you very much!

@johankool
Copy link

Great answer! Would be great to add to the documentation.

In this example it's easy to keep the properties of Player and PersistedPlayer in sync but that won't scale. Would it make sense to define something like this protocol and generic?

public protocol Persistable: Codable, FetchableRecord, MutablePersistableRecord {
  associatedtype ID: Hashable, DatabaseValueConvertible
  static var databaseTableName: String { get }
}

public struct Persisted<V>: Identifiable, FetchableRecord, MutablePersistableRecord where V: Persistable {
  static public var databaseTableName: String { V.databaseTableName }
  
  public typealias ID = V.ID
  public var id: V.ID
  public var value: V
  
  public init(row: GRDB.Row) throws {
    id = row[Column("id")]
    value = try V(row: row)
  }
  
  public func encode(to container: inout GRDB.PersistenceContainer) throws {
    container[Column("id")] = id
    try value.encode(to: &container)
  }
}

struct Player: Persistable {
  typealias ID = Tagged<Player, Int64>
  var name: String
  var score: Int
}

typealias PersistedPlayer = Persisted<Player>

func foo(dbQueue: DatabaseQueue) throws {
  try dbQueue.write { db in
    // Insert
    var player = Player(name: "Arthur", score: 100)
    let persistedPlayer = try player.insertAndFetch(db, as: PersistedPlayer.self)!
    
    let persistedName = persistedPlayer.value.name
  }
}

@groue
Copy link
Owner

groue commented Oct 20, 2023

Sure @johankool, go ahead if your app can profit from this Persisted wrapper :-)

For extra convenience, you may want to make it @dynamicMemberLookup:

@dynamicMemberLookup
struct Persisted<V> ... {
    subscript<T>(dynamicMember keyPath: KeyPath<V, T>) -> T {
        value[keyPath: keyPath]
    }
    
    subscript<T>(dynamicMember keyPath: WritableKeyPath<V, T>) -> T {
        get { value[keyPath: keyPath] }
        set { value[keyPath: keyPath] = newValue }
    }
}

// No `value` in sight!
let persistedName = persistedPlayer.name

@DandyLyons
Copy link

DandyLyons commented Dec 7, 2023

To clarify, does the Identifiable conflict exist even if the autoIncrementedPrimaryKey is something other than id? For example:

try db.create(table: "player") { t in
    t.autoIncrementedPrimaryKey("player")
}

then we could have a different property for the ID. Maybe something like this:

struct Player: Identifiable, Codable, PersistableRecord {
    var player: Int64?
    private let _uuid: UUID
    enum Identifier: Codable, Hashable {
        case db(Int64)
        case temp(UUID)
    }
    // Use a random UUID as the id until the db creates an auto incrementing primary key
    var id: Self.Identifier {
        guard let player else { return .temp(self._uuid) }
        return .db(player)
    }
}

Would GRDB/SQLite still have a conflict even if we used an approach like this?

I guess my question is, is there a way that we could call Player.find() but use the primary key instead of the Identifiable id?

@groue
Copy link
Owner

groue commented Dec 7, 2023

Hello @DandyLyons, Using a custom identifier type is fruitful idea, yes. I've performed a few similar attempts but did not find anything that could reach the level of a recommended technique. Probably I didn't try hard enough, and fresh ideas are welcome.

It's good that you can make something work in your app. Apps are the best place for exploring new techniques :-)

I'm not personally found of toying with the schema itself, and by that I mean that I'd try to find a solution that has no impact on the schema (so that people can name their column "id" if that's what they feel like doing).

If your Identifier type provided a custom implementation of == from Equatable, it would not need to create a UUID (just make .temp not equal to itself).

And if it provided a custom implementation of DatabaseValueConvertible, it would probably become autonomous, and able to directly match an "id" column (no need for two extra properties).

I guess my question is, is there a way that we could call Player.find() but use the primary key instead of the Identifiable id?

You can call Player.find(_:key:) instead of Player.find(_:id:). The key variant accepts any database value, so a plain Int64 will do.

You can also add an extension:

// App code
extension Player {
    static func find(_ db: Database, id: Int64) throws -> Player {
        try Player.find(db, key: rowid)
    }
}

If you want to share this extension between multiple record types, define a protocol.

If you want to discuss this, please open a new discussion. That's because I'd like this issue to remain focused on answers and solutions, rather than unbounded explorations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants