groue / GRDB.swift

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

Annotating with two counts from the same table fails silently. #1559

Closed OscarApeland closed 3 weeks ago

OscarApeland commented 3 weeks ago

What did you do?

I wanted to fetch the values associated from one record, and divide them into two, counted. I've done a similar operation in the past with one count, which was fairly straightforward.

What did you expect to happen?

try! optionSet.options
    .filter(DB.OptionSetOption.Columns.flag == nil || DB.OptionSetOption.Columns.flag == DB.OptionSetOption.Special.nothing.rawValue)
    .annotated(
        with:
            DB.OptionSetOption.answers.filter(DB.Answer.Columns.question == likeQuestion.code).count.forKey("likeCount"),
            DB.OptionSetOption.answers.filter(DB.Answer.Columns.question == dislikeQuestion.code).count.forKey("dislikeCount")
    )
==> My object with the option, along with `likeCount` and `dislikeCount` set to their appropriate values.

What happened instead?

The query succeeds, but both counts are 0. If either is removed, the correct count is provided for that key.

Fix

I was able to circumvent this issue by adding a key for the table, as such:

try! optionSet.options
    .filter(DB.OptionSetOption.Columns.flag == nil || DB.OptionSetOption.Columns.flag == DB.OptionSetOption.Special.nothing.rawValue)
    .annotated(
        with:
            DB.OptionSetOption.answers.forKey("like").filter(DB.Answer.Columns.question == likeQuestion.code).count.forKey("likeCount"),
            DB.OptionSetOption.answers.forKey("dislike").filter(DB.Answer.Columns.question == dislikeQuestion.code).count.forKey("dislikeCount")
    )

I was luckily able to resolve my issue on my own, but perhaps the query that failed should throw some issue about the duplicate inferred column names? From my testing it appears the error is caused by both annotations having the initial inferred column name of answerCount.

Environment

GRDB flavor(s): GRDB GRDB version: 4b934fd Installation method: SPM Xcode version: 15 Swift version: 5.8 Platform(s) running GRDB: 5.8 macOS version running Xcode: Sonoma

groue commented 3 weeks ago

Hello @OscarApeland,

You got it exactly right 💯

This is covered in the Isolation of Multiple Aggregates section of the doc.

I personally tend to put the association forKey after the filter, and to use plural names for has-many associations. But your version is just as good.

// Split the answers association so that likes and dislikes do not interfere
let likes = DB.OptionSetOption.answers
    .filter(DB.Answer.Columns.question == likeQuestion.code)
    .forKey("likes") // <- the key for likes

let dislikes = DB.OptionSetOption.answers
    .filter(DB.Answer.Columns.question == dislikeQuestion.code)
    .forKey("dislikes") // <- the key for dislikes

try optionSet.options
    .filter(DB.OptionSetOption.Columns.flag == nil || DB.OptionSetOption.Columns.flag == DB.OptionSetOption.Special.nothing.rawValue)
    .annotated(
        with:
            likes.count.forKey("likeCount"),
            dislikes.count.forKey("dislikeCount")
    )
    ...

Without the rekeying, we're dealing with the same association, identified by its key "answers". All the merging rules described in Refining Association Requests apply. You end up with computing twice the number of answers that are both likes and dislikes: zero. Only the distinct association keys make it possible to prevent any merging of filters.


When you happen to frequently write this kind of code, some helper declarations can help.

For example, associations can be declared as static constants, but you can also use static functions:

extension DB.OptionSetOption {
    // I assume this one already exists
    static let answers = hasMany(DB.Answer.self)

    /// The association to answers with a specific question code.
    static func answers(for question: QuestionCode)
    -> HasManyAssociation<DB.OptionSetOption, DB.Answer>
    {
        answers
            .filter(DB.Answer.Columns.question == question)
            .forKey("\(question)Answers") // <- unique key for this question code
    }
}

// Usage
try optionSet.options
    .filter(DB.OptionSetOption.Columns.flag == nil || DB.OptionSetOption.Columns.flag == DB.OptionSetOption.Special.nothing.rawValue)
    .annotated(
        with:
            DB.OptionSetOption
                .answers(for: likeQuestion.code)
                .count.forKey("likeCount"),
            DB.OptionSetOption
                .answers(for: dislikeQuestion.code)
                .count.forKey("dislikeCount")
    )
    ...

I'm not saying you should do this. But it might help understanding how it works.

groue commented 3 weeks ago

but perhaps the query that failed should throw some issue about the duplicate inferred column names?

Last time I tried, I found difficulties detecting this while still supporting valid cases of merges. This is an interesting idea of improvement, though 👍

OscarApeland commented 3 weeks ago

That convention is neater, thanks for the tip. I clearly wasn't aware of that documentation article, sorry about that, I'll just close this then. :)

OscarApeland commented 3 weeks ago

By the way, have you considered creating a custom GPT for this project? ChatGPT out-of-the-box is pretty useless when it comes to debugging/helping with GRDB, perhaps putting the knowledge base into a chat interface could help some users (like me).

groue commented 3 weeks ago

It would be a nice contribution indeed to build a GPT that feeds from GRDB documentation, demo apps, issues, and discussions.