groue / GRDB.swift

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

Issue with aggregate #1454

Closed ahartman closed 12 months ago

ahartman commented 1 year ago

What did you do?

Coded a tech with a count aggregate

What did you expect to happen?

Count aggregate is wrong when combined with a filter on date

What happened instead?

See screen images

Environment

GRDB flavor(s): GRDB GRDB version: master Installation method: (CocoaPods, SPM, manual?) Xcode version: Swift version: Platform(s) running GRDB: (iOS, macOS, watchOS?) macOS version running Xcode:

Demo Project

ahartman commented 1 year ago

Dear Gwendal,

I am having some subtle issues, either with SQL or with GRDB. I am working on an app for a psychology practice; the parent-child data object is patient and their visits. When retrieving a data set I also want to use an aggregate, for the count of the number of visits per patient and I want to filter the lists for a data range.

Without filtering, the visitCount is correct With filtering for a data range, the visitCount is very wrong and it seems date filtering does not work at all. The SQL looks OK for me.

Of course, I could calculate the visitCount easily in Swift after fetching but I believe in calculating as early as possible, i.e. when fetching.

Can you tell me my error? I attach two screen images, one of visitCount and one of visitCount plus date range.

Thanks in advance, André Hartman

Schermafbeelding 2023-11-07 om 07 02 51 Schermafbeelding 2023-11-07 om 07 16 19
ahartman commented 1 year ago

I also think that the example in the documentation on filtering associations many not be correct: let request = Author .including(all: Author.book .filter(Column("kind") == "novel") .forKey("novels")) .including(all: Author.book .filter(Column("kind") == "poems") .forKey("poems"))

Should it not be: "Author.books"?

Regards, André Hartman

groue commented 1 year ago

Hello @ahartman,

Without filtering, the visitCount is correct With filtering for a data range, the visitCount is very wrong.

I assume that you are surprised that the visits mentioned in annotated(with: Patient.visits.count) do not inherit the filtering of the including(all: Patient.visits.filter...).

In the end you get the count of all visits, greater than the number of elements in the array of filtered visits:

func getPatientsVisits(dates: PeriodStartEd) throws -> [PatientInfo] {
    try dbQueue.read { db in
        try Patient
            // The count of all visits
            .annotated(with: Patient.visits.count)
            // An array of filtered visits
            .including(all: Patient.visits
                .filter(Column("date") >= dates.start)
                .filter(Column("date") <= dates.end))
            .asRequest(of: PatientInfo.self)
            .fetchAll(db)
    }
}

I you want both to match, you need to count the filtered visits, as below:

func getPatientsVisits(dates: PeriodStartEd) throws -> [PatientInfo] {
    try dbQueue.read { db in
        let overlappingVisits = Patient.visits
            .filter(Column("date") >= dates.start)
            .filter(Column("date") <= dates.end)
        return try Patient
            .annotated(with: overlappingVisits.count)
            .including(all: overlappingVisits)
            .asRequest(of: PatientInfo.self)
            .fetchAll(db)
    }
}

I also think that the example in the documentation on filtering associations many not be correct [...] Should it not be: "Author.books"?

Certainly, thank you 👍 Don't hesitate opening a pull request 🙂

ahartman commented 1 year ago

Gwendal,

The example was not fully complete, probably an incorrect copy/paste: try dbQueue.read { db in let overlappingVisits = Patient.visits .filter(Column("date") >= dates.start) .filter(Column("date") <= dates.end) patientInfo = try Patient .annotated(with: overlappingVisits.count) .including(all: overlappingVisits .filter(Column("date") >= dates.start) .filter(Column("date") <= dates.end) ) .asRequest(of: PatientInfo.self) .fetchAll(db) }

That is at least a correct result but not what I expected: What I expected was the patients with visits in the filtered date range. What I get is all patients, including with a visitCount = 0 and no visits in the date range.

Any ideas? Regards, André Hartman

groue commented 1 year ago

What I expected was the patients with visits in the filtered date range. What I get is all patients, including with a visitCount = 0 and no visits in the date range.

Thanks for being explicit 😅 It's always good to be as explicit as possible - words like "wrong" do not mean anything.

The annotated and including(all:) methods never filter any result out. This explains why you get all patients, including those with an empty arrays of visits.

If you want to filter results based on the value of an association aggregate (such as Patient.visits.count), you need to use having:

func getPatientsVisits(dates: PeriodStartEd) throws -> [PatientInfo] {
    try dbQueue.read { db in
        // Define a subset of the Patient.visits association,
        // restricted to visits overlapping the date range.
        // At this point, no database access is performed.
        let overlappingVisits = Patient.visits
            .filter(Column("date") >= dates.start)
            .filter(Column("date") <= dates.end)

        // Now fetch all patients...
        return try Patient
            // ... only those with overlapping visits...
            .having(overlappingVisits.isEmpty == false)
            // ... don't forget the overlapping visits in the results...
            .including(all: overlappingVisits)
            // ... and decode in the dedicated struct:
            .asRequest(of: PatientInfo.self)
            .fetchAll(db)
    }
}

You use having instead of filter when you exclude some values based on an association aggregate, just as in SQL you'd use HAVING instead of WHERE for conditions based on SQL aggregates.

Note that I removed the annotation (since you can get the count by querying the array directly):

struct PatientInfo: Decodable, FetchableRecord {
    var patient: Patient
    var visits: [Visit]
    var visitCount: Int { visits.count }
}
ahartman commented 1 year ago

Dear Gwendal,

I now remember that SQL is very subtle as are the errors that one can get: Your latest example did not provide a visitCount at all, so I went back to doing that in the query. However, with or without visitCount, I get duplicate visits, each patient has the same number of duplicated visits.

func getPatientsVisits(dates: PeriodStartEnd) -> [PatientInfo] {
        print(dates)
        var patientInfo = [PatientInfo]()
        do {
            try dbQueue.read { db in
                let overlappingVisits = Patient.visits
                    .filter(Column("date") >= dates.start)
                    .filter(Column("date") <= dates.end)
                patientInfo = try Patient
                    .annotated(with: overlappingVisits.count)
                    .having(overlappingVisits.isEmpty == false)
                    .including(all: overlappingVisits)
                    .asRequest(of: PatientInfo.self)
                    .fetchAll(db)
                print("patientInfo: \(patientInfo.count)")
            }
        } catch {
            fatalError("Unresolved error \(error)")
        }
        return patientInfo
    }
}

struct PatientInfo: Decodable, Hashable, FetchableRecord {
    var patient: Patient
    var visitCount: Int
    var visits: [Patient.Visit]
    //var visitCount: Int {visits.count}
}

Regards, André Hartman

Schermafbeelding 2023-11-07 om 10 43 32 Schermafbeelding 2023-11-07 om 10 46 47
groue commented 1 year ago

I'm spending too much time guessing what you're after, so let's change tactics.

Please provide a minimal reproducing code, along with precise description of the expected results vs. the observed results.

For example:

// Prepare MINIMAL schema.
// Only tables and columns that are DIRECTLY related to the issue.
let dbQueue = try DatabaseQueue()
try dbQueue.write { db in
    try db.create(table: "patient") { t in ... }
    try db.create(table: "visit") { t in ... }
}

// Define MINIMAL records.
// Only types, properties and associations that are DIRECTLY
// related to the issue.
struct Patient { ... }
struct Visit { ... }
struct PatientInfo { ... }

// Prepare MINIMAL database content.
// Only rows that are DIRECTLY related to the issue.
try dbQueue.write { db in
    try Patient(...).insert(db)
    try Visit(...).insert(db)
}

// Now show the issue
// 1. Show the request.
// 2. Show the results.
// 3. Show the expected results.
// 4. No missing information.
try dbQueue.read { db in
    let results = try Patient...
}

Please be MINIMAL. I won't perform this work for you. I must be able to:

  1. paste the code.
  2. run the code and see what you are describing.
  3. without losing time deciphering what's relevant and what's not.

NO SCREENSHOT.

Finally, when you put Swift code in a GitHub issue, remember to wrap it in triple backticks, for a nicer output:

```swift
// That's Swift code
print("Hello, world!")
```
// That's Swift code
print("Hello, world!")

Thanks!

ahartman commented 1 year ago

Gwendal,

You are right, you should only have to spend minimal time on issues. I will prepare a minimal file for you.

Regards, André Hartman

ahartman commented 1 year ago

Gwendal,

Below you find a minimal code example 'DBModel1'. I commented out everything not absolutely necessary.

What you should do: Put a breakpoint on DBModel1, line 158 to observe the values of 'patientInfo' Run DBModel1.makeDB() and then DBModel1.updateFromEvents

What I would expect: For patient A 2 out of 4 visits within the date range For patient B 1 out of 2 visits within the date range For patient C 0 out of 0 visits within the date range

The result is only correct for patient C, for the others with visits inside as well as outside the date range, the date range seems not to work. The commented filter syntax also does not work.

You have made a very powerful framework; however, due to the power, it is not easy to make it work. Hope you can help and thanks in advance.

Regards, André Hartman

//
//  DBModel1.swift
//  AgendaAssistent
//
//  Created by André Hartman on 19/10/2023.
//  Copyright © 2023 André Hartman. All rights reserved.
//

import Contacts
import Foundation
import GRDB
import MapKit

var dbQueue = DBModel1().getDB()

class DBModel1 {
    var model = MainModel.shared
    public func getDB() -> DatabaseQueue {
        do {
            let fm = FileManager.default
            let path = try fm.url(for: .documentDirectory, in: .allDomainsMask, appropriateFor: nil, create: false)
            let databaseURL = path.appendingPathComponent("db.sqlite")
            NSLog("Database stored at \(databaseURL.path)")
            var dbQueue = try DatabaseQueue(path: databaseURL.path)
            var config = Configuration()
            config.prepareDatabase { db in
                db.trace { print($0) }
            }
            dbQueue = try DatabaseQueue(path: databaseURL.path, configuration: config)
            return dbQueue
        } catch {
            fatalError("Unresolved error \(error)")
        }
    }

    public func makeDB() {
        do {
            try dbQueue.write { db in
                try db.create(table: "patient", ifNotExists: true) { t in
                    t.autoIncrementedPrimaryKey("id")
                    t.column("patientName", .text)
                        .indexed()
                        .unique()
                        .notNull()
                }
            }
            try dbQueue.write { db in
                try db.create(table: "visit", ifNotExists: true) { t in
                    t.column("id", .text)
                        .primaryKey()
                        .indexed()
                    t.column("created", .text)
                        .notNull()
                    t.column("date", .text)
                        .notNull()
                        .indexed()
                    t.belongsTo("patient", onDelete: .cascade)
                }
            }
        } catch {
            fatalError("Unresolved error \(error)")
        }
    }

    func updateFromEvents() {
        let df = DateFormatter()
        df.dateFormat = "yyyy-MM-dd"
        let localVisits = [
            EventsLine(
                id: "61CACEB7-CCF5-4942-914E-AF85F3601CF2",
                patientName: "patient A",
                dateVisit: df.date(from: "2023-11-08")!,
                createdVisit: df.date(from: "2023-11-01")!
            ),
            EventsLine(
                id: "AC24CB24-C5A5-4895-9726-724EF436A4B1",
                patientName: "patient A",
                dateVisit: df.date(from: "2023-11-07")!,
                createdVisit: df.date(from: "2023-11-01")!
            ),
            EventsLine(
                id: "0B069AE7-8A49-48CE-BB12-A5ED829F12D5",
                patientName: "patient A",
                dateVisit: df.date(from: "2023-10-05")!,
                createdVisit: df.date(from: "2023-10-01")!
            ),
            EventsLine(
                id: "0C747E42-2927-4CC4-9C5F-4CC684BC4C52",
                patientName: "patient A",
                dateVisit: df.date(from: "2023-12-05")!,
                createdVisit: df.date(from: "2023-10-01")!
            ),
            EventsLine(
                id: "1A9C7660-6A2D-46CA-9625-91917B5B47BE",
                patientName: "patient B",
                dateVisit: df.date(from: "2023-11-06")!,
                createdVisit: df.date(from: "2023-11-01")!
            ),
            EventsLine(
                id: "8B8A3776-36F2-441E-859F-771EFAF0990F",
                patientName: "patient B",
                dateVisit: df.date(from: "2023-10-04")!,
                createdVisit: df.date(from: "2023-10-01")!
            ),
            EventsLine(
                id: "3019BBF2-DC31-4B08-BBFA-2530C10B2F4B",
                patientName: "patient C",
                dateVisit: df.date(from: "2021-10-04")!,
                createdVisit: df.date(from: "2021-10-01")!
            )
        ]
        do {
            try dbQueue.inTransaction { db in
                for visit in localVisits {
                    var patient = Patient(
                        patientName: visit.patientName
                    )
                    patient = try patient.upsertAndFetch(db)

                    var tempVisit = Patient.Visit(
                        id: UUID(uuidString: visit.id)!,
                        created: visit.createdVisit,
                        date: visit.dateVisit,
                        patientID: patient.id!
                    )
                    try tempVisit.upsert(db)
                }
                return .commit
            }
        } catch {
            fatalError("Unresolved error \(error)")
        }

        let dateStart = df.date(from: "2022-11-06")
        let dateEnd = df.date(from: "2024-11-13")
        var patientInfo = [PatientInfo]()
        do {
            try dbQueue.read { db in
                /*
                 let overlappingVisits = Patient.visits
                 .filter(Column("date") >= dateStart)
                 .filter(Column("date") <= dateEnd)
                 */
                patientInfo = try Patient
                    //.annotated(with: Patient.visits.count)
                    //.having(Patient.visits.isEmpty == false)
                    .including(all: Patient.visits
                        .filter((dateStart!...dateEnd!).contains(Column("date")))
                        //.filter(Column("date") >= dateStart)
                        //.filter(Column("date") <= dateEnd)
                    )
                    .asRequest(of: PatientInfo.self)
                    .fetchAll(db)
            }
        } catch {
            fatalError("Unresolved error \(error)")
        }
        print(patientInfo)
    }
}

struct PatientInfo: Decodable, Hashable, FetchableRecord {
    var patient: Patient
    //var visitCount: Int
    var visits: [Patient.Visit]
}

struct Patient: Codable, Hashable, FetchableRecord, MutablePersistableRecord, TableRecord, EncodableRecord {
    var id: Int?
    var patientName: String
    static let visits = hasMany(Visit.self)

    struct Visit: Codable, Hashable, FetchableRecord, MutablePersistableRecord, TableRecord, EncodableRecord {
        var id: UUID
        var created: Date
        var date: Date
        var patientID: Int
        static let patient = belongsTo(Patient.self)
    }
}

struct EventsLine: Hashable, Identifiable {
    var id: String
    var patientName: String
    var dateVisit: Date
    var createdVisit: Date
}
ahartman commented 1 year ago

Dear Gwendal,

I also noted last night that the rowids of the patient records are 1, 5 and 7, i.e. the sequence increases with the child inserts as well. A previous verision does not do that and numbers 1, 2, 3 as I would expect and I can no difference at all in the code. Idea?

Regards, André Hartman

groue commented 1 year ago

Thanks André, I will look at your sample code shortly!

ahartman commented 1 year ago

Dear Gwendal,

I suggest you take no more than a quick look.

Yesterday suddenly the results were correct; in a given situation I fetched one visit for a patient in a timeframe, which was correct. An hour later, I got 3 duplicates of that single visit, all with the same date but all with a different UUID as primary key!

Weirdly inconsistent results, re-installed Xcode and I suspect something is broken in my environment. I currently develop on an 8-year old iMac, still on Monterey. I have a new iMac M3 on order, and I will pause development until the new machine has arrived.

Regards, André Hartman

groue commented 1 year ago

The result is only correct for patient C, for the others with visits inside as well as outside the date range.

I assume that the following lines must be appended to your sample code:

DBModel1().makeDB()
DBModel1().updateFromEvents()

I don't see any problem.


Modifying the trace as below reveals actual values, and helps debugging:

class DBModel1 {
    // Commented out because this lines prevents the sample code from compiling
    // var model = MainModel.shared
    public func getDB() -> DatabaseQueue {
        do {
            // Removed all the file system stuff,
            // useless in a minimal sample code.
            var config = Configuration()
            config.prepareDatabase { db in
                // The expandedDescription shows all values
                db.trace { print($0.expandedDescription) }
            }
            return try DatabaseQueue(configuration: config)
        } catch {
            fatalError("Unresolved error \(error)")
        }
    }

I also dumped the database content before reading the patient info (requires a recent GRDB version):

try dbQueue.read { db in
    // Dump table headers so that we easily
    // distinguish the columns 'created' and 'date'
    try db.dumpContent(format: .debug(header: true))

Here is what I get:

...

patient
SELECT * FROM "patient" ORDER BY "id"
id|patientName
1|patient A
5|patient B
7|patient C

visit
SELECT * FROM "visit" ORDER BY "id"
id|created|date|patientId
0B069AE7-8A49-48CE-BB12-A5ED829F12D5|2023-09-30 22:00:00.000|2023-10-04 22:00:00.000|1
0C747E42-2927-4CC4-9C5F-4CC684BC4C52|2023-09-30 22:00:00.000|2023-12-04 23:00:00.000|1
1A9C7660-6A2D-46CA-9625-91917B5B47BE|2023-10-31 23:00:00.000|2023-11-05 23:00:00.000|5
3019BBF2-DC31-4B08-BBFA-2530C10B2F4B|2021-09-30 22:00:00.000|2021-10-03 22:00:00.000|7
61CACEB7-CCF5-4942-914E-AF85F3601CF2|2023-10-31 23:00:00.000|2023-11-07 23:00:00.000|1
8B8A3776-36F2-441E-859F-771EFAF0990F|2023-09-30 22:00:00.000|2023-10-03 22:00:00.000|5
AC24CB24-C5A5-4895-9726-724EF436A4B1|2023-10-31 23:00:00.000|2023-11-06 23:00:00.000|1
------------------------
PRAGMA main.foreign_key_list("visit")
SELECT * FROM "patient"
SELECT *, "patientId" AS "grdb_patientId" FROM "visit"
 WHERE ("date" BETWEEN '2022-11-05 23:00:00.000' AND '2024-11-12 23:00:00.000')
   AND ("patientId" IN (1, 5, 7))
COMMIT TRANSACTION

There is no visit with a date outside of the range in the fetched results, so I don't see what the problem is.

The range is 2022-11-05 23:00:00.000...2024-11-12 23:00:00.000 (that's the UTC dates that the DateFormatter() of your sample code gives on my mac, in my time zone).

// 2023-11-07 23:00:00 +0000
// 2023-11-06 23:00:00 +0000
// 2023-10-04 22:00:00 +0000
// 2023-12-04 23:00:00 +0000
// 2023-11-05 23:00:00 +0000
// 2023-10-03 22:00:00 +0000
for patientInfo in patientInfo {
    for visit in patientInfo.visits {
        print(visit.date)
    }
}

If I modify your sample code so that one more visit is out of the range, it is no longer present in the fetched results:

 EventsLine(
     id: "0B069AE7-8A49-48CE-BB12-A5ED829F12D5",
     patientName: "patient A",
-    dateVisit: df.date(from: "2023-10-05")!,
+    dateVisit: df.date(from: "2021-10-05")!,
     createdVisit: df.date(from: "2023-10-01")!
 ),

Maybe there is a confusion with the created column? It was not removed from the minimal sample code, so maybe you think it is relevant? Anyway, it is not present in the filtering.

Maybe you have difficulties handling UTC dates?

I can't quite tell, given the sample code does not reveal any problem to look at.

Somehow I would expect that a database with a single patient and a single visit would be enough to reveal the issue you are trying to explain.

You have made a very powerful framework; however, due to the power, it is not easy to make it work.

Databases are rich beasts. And dates are another (very) complicated topic. Dealing with both can be tiring indeed.

In my current understanding of your issue, focusing on GRDB is maybe not the good strategy. I recommend focusing on SQLite itself. You are right using the trace command. Maybe dump will help you as well. Looking at the raw SQL will often help understanding what exactly is happening. If something is incorrect at the SQL level, you'll understand what has to be changed in the instructions given to GRDB.

ahartman commented 1 year ago

Dear Gwendal,

Thank you for your time. I uses an ineffective date range as you observed. My adapted minimal code below:

/// // // DBModel1.swift // AgendaAssistent // // Created by André Hartman on 19/10/2023. // Copyright © 2023 André Hartman. All rights reserved. //

import Contacts import Foundation import GRDB import MapKit

var dbQueue = DBModel1().getDB()

class DBModel1 { var model = MainModel.shared public func getDB() -> DatabaseQueue { do { let fm = FileManager.default let path = try fm.url(for: .documentDirectory, in: .allDomainsMask, appropriateFor: nil, create: false) let databaseURL = path.appendingPathComponent("db.sqlite") NSLog("Database stored at (databaseURL.path)")

        var dbQueue = try DatabaseQueue(path: databaseURL.path)
        var config = Configuration()
        config.prepareDatabase { db in
            db.trace { print($0) }
        }
        dbQueue = try DatabaseQueue(path: databaseURL.path, configuration: config)
        return dbQueue
    } catch {
        fatalError("Unresolved error \(error)")
    }
}

public func makeDB() {
    do {
        try dbQueue.erase()
        try dbQueue.write { db in
            try db.create(table: "patient", ifNotExists: true) { t in
                t.autoIncrementedPrimaryKey("id")
                t.column("patientName", .text)
                    .indexed()
                    .unique()
                    .notNull()
            }
        }
        try dbQueue.write { db in
            try db.create(table: "visit", ifNotExists: true) { t in
                t.column("id", .text)
                    .primaryKey()
                    .indexed()
                t.column("visitDate", .text)
                    .notNull()
                    .indexed()
                t.belongsTo("patient", onDelete: .cascade)
            }
        }
    } catch {
        fatalError("Unresolved error \(error)")
    }
}

func updateFromEvents() {
    let df = DateFormatter()
    df.dateFormat = "yyyy-MM-dd"
    let localVisits = [
        EventsLine(
            id: "61CACEB7-CCF5-4942-914E-AF85F3601CF2",
            patientName: "patient A",
            visitDate: df.date(from: "2023-11-08")!
        ),
        EventsLine(
            id: "AC24CB24-C5A5-4895-9726-724EF436A4B1",
            patientName: "patient A",
            visitDate: df.date(from: "2023-11-07")!
        ),
        EventsLine(
            id: "0B069AE7-8A49-48CE-BB12-A5ED829F12D5",
            patientName: "patient A",
            visitDate: df.date(from: "2023-10-05")!
        ),
        EventsLine(
            id: "0C747E42-2927-4CC4-9C5F-4CC684BC4C52",
            patientName: "patient A",
            visitDate: df.date(from: "2023-12-05")!
        ),
        EventsLine(
            id: "1A9C7660-6A2D-46CA-9625-91917B5B47BE",
            patientName: "patient B",
            visitDate: df.date(from: "2023-11-06")!
        ),
        EventsLine(
            id: "8B8A3776-36F2-441E-859F-771EFAF0990F",
            patientName: "patient B",
            visitDate: df.date(from: "2023-10-04")!
        ),
        EventsLine(
            id: "3019BBF2-DC31-4B08-BBFA-2530C10B2F4B",
            patientName: "patient C",
            visitDate: df.date(from: "2021-10-04")!
        )
    ]
    do {
        try dbQueue.inTransaction { db in
            for visit in localVisits {
                var patient = Patient(
                    patientName: visit.patientName
                )
                patient = try patient.upsertAndFetch(db)

                var tempVisit = Patient.Visit(
                    id: UUID(uuidString: visit.id)!,
                    visitDate: visit.visitDate,
                    patientID: patient.id!
                )
                try tempVisit.upsert(db)
            }
            return .commit
        }
    } catch {
        fatalError("Unresolved error \(error)")
    }

    let dateStart = df.date(from: "2023-11-06")
    let dateEnd = df.date(from: "2023-11-13")
    var patientInfo = [PatientInfo]()
    do {
        try dbQueue.read { db in
           patientInfo = try Patient
                .including(all: Patient.visits
                    //.filter((dateStart! ... dateEnd!).contains(Column("visitDate")))
                     .filter(Column("visitDate") >= dateStart)
                     .filter(Column("visitDate") <= dateEnd)
                )
                .asRequest(of: PatientInfo.self)
                .fetchAll(db)
        }
    } catch {
        fatalError("Unresolved error \(error)")
    }
    print(patientInfo)
}

}

struct PatientInfo: Decodable, Hashable, FetchableRecord { var patient: Patient var visits: [Patient.Visit] }

struct Patient: Codable, Hashable, FetchableRecord, MutablePersistableRecord, TableRecord, EncodableRecord { var id: Int? var patientName: String static let visits = hasMany(Visit.self)

struct Visit: Codable, Hashable, FetchableRecord, MutablePersistableRecord, TableRecord, EncodableRecord {
    var id: UUID
    var visitDate: Date
    var patientID: Int
    static let patient = belongsTo(Patient.self)
}

}

struct EventsLine: Hashable, Identifiable { var id: String var patientName: String var visitDate: Date } ///

The big news that the date filtering now works OK with both forms of filtering! I will never know why this created problems in the first place.

I will now start from the working example and - very carefully - add the necessary details and if it stops working, go back and find out the root cause. Thank you for your time.

Renards, André Hartman

groue commented 1 year ago

I suggest you take no more than a quick look.

That's what I did ;-)

Hold on, I'm sure you'll fix your issue. I'll help when I can.

ahartman commented 1 year ago

Dear Gwendal,

I tried retrieving patient without patients that have no visits with the .having clause that you suggested. This does not work and I see as below:

 try dbQueue.read { db in
                patientInfo = try Patient
                    .including(all: Patient.visits
                        .filter(Column("visitDate") >= dateStart)
                        .filter(Column("visitDate") <= dateEnd)
                    )
                    .having(Patient.visits.count > 0)
                    .asRequest(of: PatientInfo.self)
                    .fetchAll(db)
            }
PRAGMA query_only = 1
BEGIN DEFERRED TRANSACTION
PRAGMA main.foreign_key_list("visit")
SELECT "patient".* FROM "patient" LEFT JOIN "visit" ON "visit"."patientId" = "patient"."id" GROUP BY "patient"."id" HAVING COUNT(DISTINCT "visit"."rowid") > ?
SELECT *, "patientId" AS "grdb_patientId" FROM "visit" WHERE ("visitDate" >= ?) AND ("visitDate" <= ?) AND ("patientId" IN (?, ?, ?))
COMMIT TRANSACTION
PRAGMA query_only = 0

I think GRDB first calculates the having clause and then filters the visits on the date range. If that is correct, then the having clause will always give a wrong result?

This is simple to correct without a having clause, adding a .filter on the fetched array but that is less nice:

let temp = patientInfo.filter { $0.visits.count > 0 }

Why do I see 2 queries, should the date filtering not go into a WHERE clause in the first query?

Regards, André Hartman

groue commented 1 year ago

I think GRDB first calculates the having clause and then filters the visits on the date range. If that is correct, then the having clause will always give a wrong result?

It does not give wrong results, it gives the results the code asks for.

Please read again this comment above.

groue commented 12 months ago

@ahartman, I'm closing the issue because I think all of your questions have been addressed. Feel free to open a new issue if a new question occurs.

ahartman commented 11 months ago

Dear Gwendal,

I am well on my way to reconstructing my project based on GRDB. I have one issue, quite separate from GRDB, that you may be able to help me. The struct that GRDB uses to retrieves associated records is a bit different than my data object:

struct PatientInfo: Decodable, Hashable, FetchableRecord {
    var patient: Patient
    var visits: [Patient.Visit]
}

struct Patient: Codable, Hashable, FetchableRecord, MutablePersistableRecord, TableRecord, EncodableRecord {
    var id: Int?
    var patientName: String
    var patientLatitude: CLLocationDegrees?
    var patientLongitude: CLLocationDegrees?
    static let visits = hasMany(Visit.self)

    struct Visit: Codable, Hashable, FetchableRecord, MutablePersistableRecord, TableRecord, EncodableRecord {
        var id: String
        var visitAge: Int
        var visitCalendar: String
        var visitCreated: Date
        var visitDate: Date
        var visitFirst: Bool
        var patientID: Int
        static let patient = belongsTo(Patient.self)
    }
}

struct PatientHistory: Hashable {
    let id: String
    let patientName: String
    let patientLatitude: CLLocationDegrees?
    let patientLongitude: CLLocationDegrees?
    let patientVisits: [PatientVisit]

    struct PatientVisit: Hashable {
        let id: String
        let visitAge: Int
        let visitCalendar: String
        let visitCreated: Date
        let visitDate: Date
        let visitFirst: Bool
    }
}

From GRDB I use PatientInfo, based on Patient and Patient.Visit. My project uses the PatientHistory object, where Patient data is directly in the struct.

The best I could come up with to transform PatientInfo into PatientHistory is the ugliest code, I have written in years:

var newPatientInfo = [PatientHistory]()
        for patient in patientInfo {
            var newVisitInfo = [PatientHistory.PatientVisit]()
            for visit in patient.visits {
                newVisitInfo.append(
                    PatientHistory.PatientVisit(
                        id: visit.id,
                        visitAge: visit.visitAge,
                        visitCalendar: visit.visitCalendar,
                        visitCreated: visit.visitCreated,
                        visitDate: visit.visitDate,
                        visitFirst: visit.visitFirst
                    )
                )
            }
            newPatientInfo.append(
                PatientHistory(
                    id: String(patient.patient.id!),
                    patientName: patient.patient.patientName,
                    patientLatitude: patient.patient.patientLatitude,
                    patientLongitude: patient.patient.patientLongitude,
                    patientVisits: newVisitInfo
                )
            )
        }

Can you point to a better, shorter and more functional way? Thank you. ahartman, Belgium

groue commented 11 months ago

Hello @ahartman,

Maybe try getting rid of PatientInfo, and directly fetch PatientHistory (make it conform to FetchableRecord)?

ahartman commented 11 months ago

Dear Gwendal,

Excellent suggestion, I tried as below by modifying your example:

struct PatientInfo1: Decodable, Hashable, FetchableRecord {
    var id: Int?
    var patientName: String
    var latitude: CLLocationDegrees?
    var longitude: CLLocationDegrees?

    var visits: [Patient.Visit]
}

The fetch is as below:

func getPatientInfo1(dates: PeriodStartEnd) -> [PatientInfo1] {
        var patientInfo1 = [PatientInfo1]()
        do {
            try dbQueue.read { db in
                patientInfo1 = try Patient
                    .including(all: Patient.visits
                        .filter(Column("visitDate") >= dates.start)
                        .filter(Column("visitDate") <= dates.end)
                    )
                    .order(Column("patientName"))
                    .asRequest(of: PatientInfo1.self)
                    .fetchAll(db)
            }
        } catch {
            fatalError("Unresolved error \(error)")
        }
        patientInfo1 = patientInfo1.filter { $0.visits.count > 0 }

        return patientInfo1
    }

This works perfectly, so many thanks for the suggestion. I must say I still do not understand the dependencies between the various structs that you use; I am now surprised that PatientInfo1 works although Patient is no longer mentioned in PatientInfo1. Documentation is even more important than the actual code.

Regards, ahartman

groue commented 11 months ago

I must say I still do not understand the dependencies between the various structs that you use; I am now surprised that PatientInfo1 works although Patient is no longer mentioned in PatientInfo1. Documentation is even more important than the actual code.

Maybe it will be easier to understand if you look at the fundamental protocol of all fetched records (plain structs or composed ones):

protocol FetchableRecord {
    init(row: Row) throws
}

That's all. There's nothing else. And Row has no idea of what's it's used for.

This means that the only way to decode a row is, at some point, to use numerical indexes or strings.

extension Player: Decodable, FetchableRecord {
    var id: Int64
    var name: String
    var score: Int

    init(row: Row) {
        // Usually generated for you. But there's no magic:
        // for Decodable records, we use the strings stored
        // in the generated CodingsKeys enum.
        id = row[0]
        name = row["name"]
        score = row[CodingKeys.score.stringValue]
    }
}

let players = try Player.fetchAll(db)

It is the same for associated records:

struct PlayerInfo: Decodable, FetchableRecord {
    var player: Player
    var team: Team

    init(row: Row) {
        // Again, this is usually generated for you.
        // But it doesn't get any more magical. We
        // still need the strings stored in the
        // CodingsKeys enum at some point.
        player = Player(row: row)
        team = row[CodingKeys.team.stringValue]
    }
}

let playerInfos = try Player
    .including(required: Player.team)
    .asRequest(of: PlayerInfo.self)
    .fetchAll(db)

Row does not care where those strings come from. As long as we can find a column named "patientName", and an array of associated records names "visits", we can decode the row. That those strings come from Patient.CodingKeys, or PatientInfo1.CodingKeys, is irrelevant.

The little subtlety that I did not disclose yet is that in a composed record, the name of the property that contains the "root" record is ignored:

struct PlayerInfo1: Decodable, FetchableRecord {
    var player: Player // could be named "foo" or even "pretzel"
    var team: Team

    init(row: Row) {
        // In reality, the generated code here tries to
        // find the key "player" (or "foo", or "pretzel"),
        // and does not find any. It thus assume this is
        // the root record, decoded from plain columns:
        player = Player(row: row)
        team = row[CodingKeys.team.stringValue]
    }
}

struct PlayerInfo2: Decodable, FetchableRecord {
    var name: String
    var score: Int
    var team: Team

    init(row: Row) {
        // Here it's easy. There is a "name" column,
        // a "score" column, and a "team" associated record.
        name = row[CodingKeys.name.stringValue]
        score = row[CodingKeys.score.stringValue]
        team = row[CodingKeys.team.stringValue]
    }
}

let playerInfos = try Player
    .including(required: Player.team)
    .asRequest(of: PlayerInfo1.self) // or PlayerInfo2.self
    .fetchAll(db)
ahartman commented 11 months ago

Dear Gwendal,

Thank you for your kind words but concepts like protocols are too abstract for me. I cannot really understand much of your documentation, I use the examples.

At 73 years of age and with 10 years of IOS development as a hobby, I'm a 'programmer by example'. I find something on Internet that is similar to what I want to achieve, make it work and then adapt it step by step to what I want.

Regards, André Hartman

groue commented 11 months ago

Oh, please forgive my lengthy digressions, then! Thank you for reminding me of your programming practice. I know it, I've shared it before, and I hope to find it again in the future. I'll try to write better answers in the future!