Unable to decode database NULL value as Swift nil value of type Int? when using ScopeAdapter built from splittingRowAdapters about grdb.swift HOT 13 CLOSED

Thanks for the report, @mallman. Please hold on!

from grdb.swift.

Hi again, @mallman,

Let's first understand why you're unlucky.

You say row.scopesTree[key.stringValue] is not nil. This can only happen if a scope has been defined by the key, and scopes are never defined out of the blue. There must be an explicit ScopeAdapter in your app, OR some association is involved in your decoded request, with including(required:), including(optional:), or maybe annotated(with...) - associations define scopes as well.

So something is defining a scope. And it has the same name as your key. Does this ring a bell to you?

I think there is a bug in the GRDB snippet you have mentioned, because scoped rows are usually used to decode other records, and those records are decoded as nil if the scoped row only contains NULL values.

To illustrate, let's run this code:

struct Team { }

struct Player {
    static let team = belongsTo(Team.self) // defines the "team" association key
}

struct PlayerInfo: Decodable, FetchableRecord {
    var player: Player
    var team: Team? // decoded from the "team" scope
}

let infos = try Player
    .including(optional: Player.team) // defines the "team" scope
    .asRequest(of: PlayerInfo.self)
    .fetchAll(db)

The SQL request is:

--          the "team" scope
--               <---->
SELECT player.*, team.*
FROM player
LEFT JOIN team ON team.id = player.teamId

Let's pretend the raw rows are:

                           the "team" scope
                        <--------------------> 
id: 1, name: "Arthur",  id: 1,    name: "Reds"
id: 2, name: "Barbara", id: 1,    name: "Reds"
id: 3, name: "Cali",    id: NULL, name: NULL

In all rows, row.scopesTree["team"] is not nil, because the "team" scope was added by the association.

But in the third row, the team row only contains NULL values. This means that PlayerInfo.team is (as expected) decoded as nil.

YET decodeNil(forKey: .team) returns false. And that's a problem.

Do you think I described your issue?

from grdb.swift.

I think you've hit the nail on the head. Let me give you a pared-down look at some of the record decoding in question:

struct BrowserItemRecord: Decodable, FetchableRecord {
  let stackSize: Int?
  let version: VersionRecord

  private static func adapter(_ db: Database) throws -> RowAdapter {
    let adapters = try splittingRowAdapters(columnCounts: [
      1, // stackSize
      VersionRecord.numberOfSelectedColumns(db)
    ])
    return ScopeAdapter([
      CodingKeys.stackSize.stringValue: adapters[0],
      CodingKeys.version.stringValue: adapters[1]
  }
}

I decode a BrowserItemRecord with a SQLRequest adapted by adapter(db). In my query, the stackSize column may contain NULL. When it does, I get the error I described in the issue description. As a workaround, I've modified the query to return a sentinel value instead of NULL. Buuuuut... that Swift value really should be nil sometimes. It would be cleaner and easier for GRDB to automatically take care of that for me. :)

From the way you've described it, it sounds like the issue for GRDB is it doesn't like it when a row scope has a single column? Does that sound right?

Thank you very much!

from grdb.swift.

There's a bug in GRDB, that's for sure. decodeNil returns false when it should return nil. This is not a very well tested area, so I'll tighten bolts there.

But BrowserItemRecord goes through… unexpected hoops 🤔

Look at version: VersionRecord. It is a record decoded from a scoped row named "version". That's expected, and that's what scopes are made for: they define a "subrow" made of a particular subset of columns, and it is possible to decode a record from those columns.

But stackSize: Int? is not a record. It is a plain value. Unlike version, which has properties, and expects columns that match those properties, stackSize does not expect to decode its properties from matching columns: it has no properties.

When I look at the adapter, which defines a stackSize scope, I understand that you expect BrowserItemRecord to be decoded from this JSON:

{
  // Two scopes: stackSize and version
  "stackSize": {
    // The columns in the stackSize scope
    "stackSize": 123
  },
  "version": {
    // The columns in the version scope
  }
}

This is weird. The expected JSON is instead:

{
  "stackSize": 123,
  "version": { }
}

I'm thus not sure the adapter is well-fitted to BrowserItemRecord.

I'll make some checks and come back to you. I don't have clear ideas about the expected behavior when a row contains BOTH a column named stackSize, and a scope named stackSize (I have assumed that the first column is named stackSize - please tell if I'm wrong).

from grdb.swift.

The JSON representation you present is indeed weird.

Are you suggesting that for every value count in columnCounts passed to splittingRowAdapters(columnCounts: columnCounts), count > 1? Should that be a requirement?

Let me investigate an alternative. Meanwhile, if you have any suggestions for a better way to define a row adapter (or otherwise decode a record like mine), please do.

I could define a separate record to bring together all of the single column properties, and then define an adapter that includes that record. It's something to consider.

from grdb.swift.

Are you suggesting that for every value count in columnCounts passed to splittingRowAdapters(columnCounts: columnCounts), count > 1? Should that be a requirement?

No. Sorry I wasn't clear.

It's more about nesting and grouping columns. Decoding an object named version requires a "version" key in JSON which contains the version properties (one level of nesting in JSON). That's what the "version" scope does. But decoding a value named stackSize does not require nesting in JSON, and does not require a scope.

Actually, I'd suggest to remove the "stackSize" scope. The snippet below is identical to yours, except for the defined scopes:

struct BrowserItemRecord: Decodable, FetchableRecord {
  let stackSize: Int?
  let version: VersionRecord

  private static func adapter(_ db: Database) throws -> RowAdapter {
    let adapters = try splittingRowAdapters(columnCounts: [
      1, // stackSize
      VersionRecord.numberOfSelectedColumns(db)
    ])
    return ScopeAdapter([
      // No scope for stackSize
      CodingKeys.version.stringValue: adapters[1]
    ])
  }
}

from grdb.swift.

If I come back to a toy example, and still using JSON as a comparison:

Consider the following object:

struct Membership {
    var player: Player
    var team: Team
}

We expect the following JSON:

{
  "player": {
    "id": 1,
    "name": "Arthur"
  },
  "team": {
    "id": 2,
    "name": "Reds"
  }
}

And the JSON below would not work well, because it is flat and has ambiguous keys:

{
  "id": 1,
  "name": "Arthur",
  "id": 2,
  "name": "Reds"
}

But this flat JSON is exactly what SQL provides, without any further processing:

SELECT player.*, team.* FROM ...

Raw database rows are flat, and have ambiguous columns:

- id: 1
- name: "Arthur"
- id: 2
- name: "Reds"

So we need to introduce nesting, and that's what scopes can do:

- scope "player"
  - id: 1
  - name: "Arthur"
- scope "team"
  - id: 2
  - name: "Reds"

In your BrowserItemRecord, version can profit from nesting, but stackSize not really:

- stackSize: 123
- scope "version"
  - major: 2
  - minor: 0
  - patch: 0

This would match the expected JSON:

{
  "stackSize": 123,
  "version": {
    "major": 1,
    "minor": 0,
    "patch": 0
  }
}

Things could turn more complicated in version ALSO had a property named stackSize, in which case decoding BrowserItemRecord.stackSize could require some desambiguation, and another scope. But I'm not sure this is your case.

from grdb.swift.

When debugging, you may enjoy the debugDescription property of rows:

let request = /* your request */
if let row = try Row.fetchOne(db, request) {
  print(row.debugDescription)
}

With two scopes (your initial code), I'd expect something which looks like below. See how stackSize is both a scope and a column (and this is confusing for both of us, and triggers the latent GRDB bugs I have to fix):

▿ [stackSize:123, major:1, minor:0, patch:0]
  unadapted: [stackSize:123, major:1, minor:0, patch:0]
  - stackSize: [stackSize:123]
  - version: [major:1, minor:0, patch:0]

With only one scope (version), I'd expect something like:

▿ [stackSize:123, major:1, minor:0, patch:0]
  unadapted: [stackSize:123, major:1, minor:0, patch:0]
  - version: [major:1, minor:0, patch:0]

From such a row, BrowserItemRecord would find its stackSize in the main row, and its version from the scoped row.

from grdb.swift.

The #1533 PR fixes your issue, @mallman.

Your app will happen to work as you expect, even if it misuses the stackSize scope. decodeNil will return true for the stackSize key because it contains a stackSize column that contains NULL, as it should always have done (that was the GRDB bug). This fix will help stackSize to be decoded as nil, as you expect, through the rather obscure stdlib Decodable machinery.

But I really suggest that you remove the "stackSize" scope. Your BrowserItemRecord wants to decode stackSize from a column, not from a scope. stackSize is a plain value, not a record made of several columns. Only nested records need scopes. So BrowserItemRecord only needs a scope for version:

private static func adapter(_ db: Database) throws -> RowAdapter {
  // Return an adapter that defines the "version" scope,
  // aimed at decoding the `version` record property.
  // The "version" scope is made of all `VersionRecord` columns,
  // starting after the first column (stackSize):
  let adapters = try splittingRowAdapters(columnCounts: [
    1, // stackSize
    VersionRecord.numberOfSelectedColumns(db)
  ])
  return ScopeAdapter([
    CodingKeys.version.stringValue: adapters[1]
  ])
}

If your database row is made of an initial "stackSize" columns, and all other columns feed version, you can simplify the adapter:

private static func adapter(_ db: Database) throws -> RowAdapter {
  // Return an adapter that defines the "version" scope,
  // aimed at decoding the `version` record property.
  // The "version" scope is all columns but the first (stackSize):
  ScopeAdapter([
    CodingKeys.version.stringValue: SuffixRowAdapter(fromIndex: 1)
  ])
}

from grdb.swift.

I comment much too much in this issue, @mallman, but let me share a debugging tip.

Whenever you have issues decoding a Decodable & FetchableRecord type, you can provide a debugging initializer:

struct MyRecord: Decodable, FetchableRecord {
    #warning("TODO: remove this debugging initializer")
    init(row: Row) throws {
        print(row.debugDescription)
        self = try FetchableRecordDecoder().decode(MyRecord.self, from: row)
    }
}

The debugging output can help understanding issues, or write bug reports.

from grdb.swift.

The fix has shipped in 6.27.0.

from grdb.swift.

Hi @groue. Thank you for your prompt action in fixing this bug and all of your helpful tips.

I've taken your advice to heart and have eliminated "scopes" for single column "records". That has been working for me.

I've also taken the time to revisit, reorganize and rewrite a lot of my GRDB hack code, improving its rigor and reducing complexity.

For example, I have pulled up two subqueries in a query into a join and select on two CTEs instead, and I'm using GRDB's CTE support and QueryInterfaceRequest to clean up my code, improve modularity and reduce leaky abstractions—among other code smells. For example, I'm replacing certain uses of SQLRequest with QueryInterfaceRequest, and I'm replacing some SQL text literals with structured GRDB SQL literals.

Overall, I'ver performed a large amount of housekeeping motivated by this one issue. Cheers.

from grdb.swift.

Thank you @mallman :-) Be assured I do appreciate your contributions as well!

from grdb.swift.