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

Association decoding keys not working as expected #584

Closed
ghost opened this issue Aug 1, 2019 · 6 comments
Closed

Association decoding keys not working as expected #584

ghost opened this issue Aug 1, 2019 · 6 comments
Labels
blocked Progress is impossible due to some external blocking reason enhancement support

Comments

@ghost
Copy link

ghost commented Aug 1, 2019

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
Copy link
Owner

groue commented Aug 1, 2019

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
Copy link
Author

ghost commented Aug 1, 2019

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
Copy link
Owner

groue commented Aug 1, 2019

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
Copy link
Owner

groue commented Aug 2, 2019

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

code
// 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<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: 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 {}

// ===

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
Copy link
Author

ghost commented Aug 2, 2019

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
Copy link
Owner

groue commented Aug 2, 2019

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.

@groue groue closed this as completed Aug 2, 2019
@groue groue added this to the GRDB 5 milestone Aug 2, 2019
@groue groue added the support label Aug 4, 2019
@groue groue added the blocked Progress is impossible due to some external blocking reason label May 1, 2020
@groue groue removed this from the GRDB 5 milestone May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Progress is impossible due to some external blocking reason enhancement support
Projects
None yet
Development

No branches or pull requests

1 participant