groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.69k stars 686 forks source link

Association decoding keys not working as expected #584

Closed ghost closed 5 years ago

ghost commented 5 years ago

What did you do?

Added a hasMany association and built and ran...

Swift.DecodingError.keyNotFound(CodingKeys(stringValue: "drivers", intValue: nil), Swift.DecodingError.Context(codingPath: [], debugDescription: "Available keys for prefetched rows: [\"f1.drivers\"]", underlyingError: nil))

As our tables are name-spaced (f1., tennis., football.), databaseTableName is provided on each entity struct. According to the docs, this should be used...

An eventual decoding key for the association. By default, it is destination.databaseTableName.

But it appears not to be, so I added the key explicitly to the association definition...

hasMany(Entity.F1.Driver.self, key: "f1.drivers")

But even with this I still get the following error...

Swift.DecodingError.keyNotFound(CodingKeys(stringValue: "drivers", intValue: nil), Swift.DecodingError.Context(codingPath: [], debugDescription: "Available keys for prefetched rows: [\"f1.drivers\"]", underlyingError: nil))

However, explicitly providing the CodingKeys enum on the info struct results in the entities being fetched and decoded correctly...

extension Entity.F1 {
  struct ConstructorInfo {
    enum CodingKeys: String, CodingKey {
      case constructor
      case drivers = "f1.drivers"
    }

    let constructor: Constructor
    let drivers: [Driver]
  }
}

What did you expect to happen?

As per the docs, databaseTableName should be used in the first instance, but explicitly providing key should also work.

Environment

GRDB flavor(s): GRDB GRDB version: 4.0.1 Installation method: CocoaPods Xcode version: 10.3 Swift version: 5.0.1 Platform(s) running GRDB: iOS macOS version running Xcode: 10.14.6

groue commented 5 years ago

Hello @micpringle,

I'd feel more sure of my interpretation of your issue with a reproducible sample code, but here it is:

You namespace your tables (maybe because you attach several databases on a single connection). You want to run SQL queries like SELECT * FROM f1.drivers.

You thus declare the databaseTableName static property on your record types, so that they look into the namespaces tables:

extension Entity.F1.Driver: TableRecord {
    static let databaseTableName = "f1.drivers"
}

This works well for plain queries (SELECT * FROM f1.drivers), but not with associations, because GRDB, by default, wants your decodable structs to have a f1.drivers key, instead of a plain drivers key.

You thus provide an explicit key to associations:

hasMany(Entity.F1.Driver.self, key: "f1.drivers")

(This is good that you have found a workaround).

Now, you read the doc:

An eventual decoding key for the association. By default, it is destination.databaseTableName.

And you interpret it as meaning that association keys should be stripped from their namespace prefix

Is this correct?

ghost commented 5 years ago

Hey @groue–thanks for the prompt response. Your interpretation is close, but not quite there. 😃

The tables are namespaced within a single db because the app covers several sports, and the sports can have clashing entities. i.e. both Tennis and F1 have person, but they have very different structures and therefore cannot be reused across both sports (different 3rd party APIs providing the data). So we end up with f1.person and football.person. This works well.

Because of this I declared databaseTableName on each struct so that the Swift struct Entity.F1.Person maps to the table f1.person, and Entity.Tennis.Person maps to the table tennis.person etc.

However, this appears to cause issues with associations.

The docs state the following about the key parameter when declaring an association...

By default, it is destination.databaseTableName

...which I expected meant that databaseTableName would be used, rather than the struct name, when executing a request with an association. But this doesn't appear to be the case.

Even though I explicitly provide databaseTableName...

extension Entity.F1.Driver: TableRecord {
  static let databaseTableName = "f1.driver"
}

...you can see from the error that GRDB is looking for drivers rather than f1.drivers when executing the association...

Swift.DecodingError.keyNotFound(CodingKeys(stringValue: "drivers", intValue: nil), Swift.DecodingError.Context(codingPath: [], debugDescription: "Available keys for prefetched rows: [\"f1.drivers\"]", underlyingError: nil))

OK, so the default behaviour isn't working as expected but I can explicitly provide the decoding key on the association using the key parameter, as follows...

hasMany(Entity.F1.Driver.self, key: "f1.drivers")

...but this generates the same error as above. So neither the default behaviour, nor explicitly providing the key appear to work as expected.

The only thing I can get to work is directly defining CodingKeys on the info object, and setting the keys strings'...

extension Entity.F1 {
  struct ConstructorInfo {
    enum CodingKeys: String, CodingKey {
      case constructor
      case drivers = "f1.drivers"
    }

    let constructor: Constructor
    let drivers: [Driver]
  }
}

What's even more curious is the error differs depending on the type of association. A hasMany generates the error above, whereas the opposite end of that relationship, the belongsTo generates the following...

Fatal error: could not read String from missing column `color` (row: [id:1 constructorId:1 name:"Hamilton, Lewis" points:223])

For completeness, here are the entities in their entirety...

// MARK: Constructor

extension Entity.F1 {
  struct Constructor {
    let id: Int
    let name: String
    let color: String
    let points: Int
  }
}

extension Entity.F1.Constructor {
  init(_ response: API.F1.Constructor) {
    self.id = response.id
    self.name = response.name
    self.color = response.colour
    self.points = response.points
  }
}

extension Entity.F1.Constructor: Codable {}

extension Entity.F1.Constructor: TableRecord, FetchableRecord, PersistableRecord {
  static let databaseTableName = "f1.constructor"
  static let drivers = hasMany(Entity.F1.Driver.self)

  var drivers: QueryInterfaceRequest<Entity.F1.Driver> {
    return request(for: Entity.F1.Constructor.drivers)
  }
}

// MARK: ConstructorInfo

extension Entity.F1 {
  struct ConstructorInfo {
    enum CodingKeys: String, CodingKey {
      case constructor
      case drivers = "f1.drivers"
    }

    let constructor: Constructor
    let drivers: [Driver]
  }
}

extension Entity.F1.ConstructorInfo: Decodable {}

extension Entity.F1.ConstructorInfo: FetchableRecord {}

// MARK: Driver

extension Entity.F1 {
  struct Driver {
    let id: Int
    let constructorId: Int
    let name: String
    let points: Int
  }
}

extension Entity.F1.Driver {
  init(_ response: API.F1.Driver) {
    self.id = response.id
    self.constructorId = response.teamId
    self.name = response.name
    self.points = response.points
  }
}

extension Entity.F1.Driver: Codable {}

extension Entity.F1.Driver: TableRecord, FetchableRecord, PersistableRecord {
  static let databaseTableName = "f1.driver"
  static let constructor = belongsTo(Entity.F1.Constructor.self)

  var constructor: QueryInterfaceRequest<Entity.F1.Constructor> {
    return request(for: Entity.F1.Driver.constructor)
  }
}

// MARK: DriverInfo

extension Entity.F1 {
  struct DriverInfo {
    enum CodingKeys: String, CodingKey {
      case constructor = "f1.constructor"
      case driver
    }
    let constructor: Constructor
    let driver: Driver
  }
}

extension Entity.F1.DriverInfo: Decodable {}

extension Entity.F1.DriverInfo: FetchableRecord {}

And an example request...

let drivers = try Entity.F1.Driver
  .including(required: Entity.F1.Driver.constructor)
  .order(Column("points").desc)
  .asRequest(of: Entity.F1.DriverInfo.self)
  .fetchAll(db)
groue commented 5 years ago

Thanks @micpringle for all those details!

All right, I'll have a close look. The management of association keys has been completely re-done in GRDB 4, due to the necessity of key inflection depending on the association usage ("children" / "child" / "childCount"). This is the part I'm the less confident about. And future breaking changes in this area, except obvious bugfixes, are basically forbidden, even in future major versions. It would just be too hard for users to spot where keys no longer match their dedicated Decodable records. I could vet it the most I could with tests and my own usage of the lib, but I also know you and other users are just so insanely wild. So it looks like you have found a blind spot! This issue will also be a good opportunity to introduce tests dedicated to namespaced tables, which are totally missing today.

See you soon with a more conclusive answer!

groue commented 5 years ago

Hello @micpringle, I have played with your code in a playground, and have two things to say.

code ```swift // To run this playground, select and build the GRDBOSX scheme. import GRDB var configuration = Configuration() configuration.trace = { print($0) } let dbQueue = DatabaseQueue(configuration: configuration) try! dbQueue.write { db in try db.create(table: "f1.constructor") { t in t.column("id", .integer).primaryKey() t.column("name", .text) t.column("color", .text) t.column("points", .integer) } try db.create(table: "f1.driver") { t in t.column("id", .integer).primaryKey() t.column("constructorId", .integer).references("f1.constructor") t.column("name", .text) t.column("points", .integer) } } // ==== enum Entity { enum F1 { } } // ==== extension Entity.F1 { struct Constructor { let id: Int let name: String let color: String let points: Int } } extension Entity.F1.Constructor: Codable {} extension Entity.F1.Constructor: TableRecord, FetchableRecord, PersistableRecord { static let databaseTableName = "f1.constructor" static let drivers = hasMany(Entity.F1.Driver.self) var drivers: QueryInterfaceRequest { return request(for: Entity.F1.Constructor.drivers) } } // MARK: ConstructorInfo extension Entity.F1 { struct ConstructorInfo { enum CodingKeys: String, CodingKey { case constructor case drivers = "f1.drivers" } let constructor: Constructor let drivers: [Driver] } } extension Entity.F1.ConstructorInfo: Decodable {} extension Entity.F1.ConstructorInfo: FetchableRecord {} // MARK: Driver extension Entity.F1 { struct Driver { let id: Int let constructorId: Int let name: String let points: Int } } extension Entity.F1.Driver: Codable {} extension Entity.F1.Driver: TableRecord, FetchableRecord, PersistableRecord { static let databaseTableName = "f1.driver" static let constructor = belongsTo(Entity.F1.Constructor.self) var constructor: QueryInterfaceRequest { return request(for: Entity.F1.Driver.constructor) } } // MARK: DriverInfo extension Entity.F1 { struct DriverInfo { enum CodingKeys: String, CodingKey { case constructor = "f1.constructor" case driver } let constructor: Constructor let driver: Driver } } extension Entity.F1.DriverInfo: Decodable {} extension Entity.F1.DriverInfo: FetchableRecord {} // === try dbQueue.write { db in try Entity.F1.Constructor(id: 1, name: "Fire", color: "red", points: 1).insert(db) try Entity.F1.Driver(id: 1, constructorId: 1, name: "Bob", points: 1).insert(db) let request = Entity.F1.Driver .including(required: Entity.F1.Driver.constructor) .order(Column("points").desc) let drivers = try request .asRequest(of: Entity.F1.DriverInfo.self) .fetchAll(db) if let row = try request.asRequest(of: Row.self).fetchOne(db) { print(row.debugDescription) print(row.scopes["f1.constructor"]!.debugDescription) } } ```

First, GRDB behaves as documented.

Let's take your example request:

let request = try Entity.F1.Driver
    .including(required: Entity.F1.Driver.constructor)
    .order(Column("points").desc)

The Entity.F1.Driver.constructor association is defined with its default key, which is Entity.F1.Constructor.databaseTableName: f1.constructor, as documented.

A useful debugging technique in order to see the available keys in the fetched results is to fetch raw rows, and print their debugDescription. The fetched rows indeed define the f1.constructor key:

// ▿ [id:1 constructorId:1 name:"Bob" points:1]
//   unadapted: [id:1 constructorId:1 name:"Bob" points:1 id:1 name:"Fire" color:"red" points:1]
//   - f1.constructor: [id:1 name:"Fire" color:"red" points:1]
//
// ▿ [id:1 name:"Fire" color:"red" points:1]
//   unadapted: [id:1 constructorId:1 name:"Bob" points:1 id:1 name:"Fire" color:"red" points:1]
if let row = try request.asRequest(of: Row.self).fetchOne(db) {
    print(row.debugDescription)
    print(row.scopes["f1.constructor"]!.debugDescription)
}

But applications are rarely interested in raw rows: instead they declare Decodable records that are designed to feed from those rows:

extension Entity.F1 {
    struct DriverInfo {
        enum CodingKeys: String, CodingKey {
            case constructor = "f1.constructor"
            case driver
        }
        let constructor: Constructor
        let driver: Driver
    }
}

Do you need to customize the CodingKeys enum? Yes you do, because without it, Decodable (not GRDB) would look for the constructor key in a row that defines the f1.constructor key. That would be a mismatch. This explains the Swift.DecodingError.keyNotFound error you see. In order to customize the decoding code synthesized by Decodable, you customize the CodingKeys enum.

The coding keys of decoded records must match the structure of requests. You have several possible strategies:

  1. Customize coding keys so that they match the association keys

    extension Entity.F1.Driver: TableRecord, FetchableRecord, PersistableRecord {
        // Association key "f1.constructor"
        static let constructor = belongsTo(Entity.F1.Constructor.self)
    }
    
    extension Entity.F1 {
        struct DriverInfo {
            enum CodingKeys: String, CodingKey {
                // Customized in order to match the association key
                case constructor = "f1.constructor"
                case driver
            }
            let constructor: Constructor
            let driver: Driver
        }
    }
    
    let request = try Entity.F1.Driver
        .including(required: Entity.F1.Driver.constructor)
        .asRequest(of: Entity.F1.DriverInfo.self)
  2. Customize association keys so that they match the coding keys

    extension Entity.F1.Driver: TableRecord, FetchableRecord, PersistableRecord {
        // Customized in order to match the record key
        static let constructor = belongsTo(Entity.F1.Constructor.self, key: "constructor")
    }
    
    extension Entity.F1 {
        struct DriverInfo {
            // Coding key "constructor"
            let constructor: Constructor
            let driver: Driver
        }
    }
    
    let request = try Entity.F1.Driver
        .including(required: Entity.F1.Driver.constructor)
        .asRequest(of: Entity.F1.DriverInfo.self)
  3. Customize the request so that it compensate for mismatching association and coding keys

    extension Entity.F1.Driver: TableRecord, FetchableRecord, PersistableRecord {
        // Association key "f1.constructor"
        static let constructor = belongsTo(Entity.F1.Constructor.self)
    }
    
    extension Entity.F1 {
        struct DriverInfo {
            // Coding key "constructor"
            let constructor: Constructor
            let driver: Driver
        }
    }
    
    // Customize the association for the request
    let constructor = Entity.F1.Driver.constructor.forKey("constructor")
    let request = try Entity.F1.Driver
        .including(constructor)
        .asRequest(of: Entity.F1.DriverInfo.self)
  4. Encapsulate the previous strategy (remove all string literals)

    extension Entity.F1.DriverInfo {
        static let constructorKey = CodingKeys.constructor.stringValue
    }
    
    extension QueryInterfaceRequest where RowDecoder == Entity.F1.Driver {
        func asDriverInfo() -> QueryInterfaceRequest<Entity.F1.DriverInfo> {
            let constructor = Entity.F1.Driver.constructor
                .forKey(Entity.F1.DriverInfo.constructorKey)
            return self
                .including(constructor)
                .asRequest(of: Entity.F1.DriverInfo.self)
        }
    }
    
    let request = try Entity.F1.Driver.all().asDriverInfo()

All those technique apply to different situations, not only namespaced tables. I admit I can not identify any one of those strategies as the best one, generally speaking. Yet, in your particular case, I would advise to use the second one, the customized association key (unless you need to write requests that feed from several namespaces...)


The second part of my answer is about the various meanings of "namespaced". You use "namespaced" because your tables are named prefix.suffix. But SQLite can also interpret SELECT * FROM prefix.suffix as feeding from the table suffix defined in the attached database named prefix. This is another meaning of "namespaced" that we have to consider as well.

This is what I'd like to think more about - but I lack time right now ;-)

ghost commented 5 years ago

Fantastic and thorough feedback–thanks @groue. I think I've totally misinterpreted which key I was supposed to be specifying on the relationship, so my sincere apologies for that.

In this instance I've gone with your recommendation and implemented 2 and everything is working as expected.

Thanks again for your prompt and insightful help with this.

–Mic

groue commented 5 years ago

Your're welcome Mic!

A reasonable change in the library could be to change the default association key so that it removes the prefix:

extension Entity.F1.Constructor: TableRecord, FetchableRecord, PersistableRecord {
    static let databaseTableName = "f1.constructor"
}

extension Entity.F1.Driver: TableRecord, FetchableRecord, PersistableRecord {
    // IN THE FUTURE: default association key "constructor"
    static let constructor = belongsTo(Entity.F1.Constructor.self)
}

I have not thought yet of the consequences of this breaking change. And it will be for GRDB 5 anyway, which is not even the embryo of a project right now. We can close this issue.