groue / GRDB.swift

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

GRDB 6 missing DatabasePool/DatabaseQueue interchangeability #1541

Closed freak4pc closed 1 month ago

freak4pc commented 2 months ago

What did you do?

I upgraded from GRDB 5.26.0 to 6.27.0, mostly seamlessly :)

What did you expect to happen?

We have some wrapper code that keeps the databaseWriter in following property:

var dbWriter: any GRDB.DatabaseWriter

Then, in our production code we use a DatabasePool, but in tests we would sometimes swap to an in-memory DatabaseQueue.

This worked fine in GRDB 5 and I hoped it would behave the same in 6.

What happened instead?

The compiler complains that certain pieces like ValueObservation or DatabaseMigrator expect a some DatabaseWriter and not any DatabaseWriter, like so:

image

Environment

GRDB flavor(s): GRDB GRDB version: 6.27.0 Installation method: SPM over Tuist Xcode version: 15.3 Swift version: 5.10 Platform(s) running GRDB: iOS macOS version running Xcode: Sonoma

Demo Project

Doesn't seem needed in this case

groue commented 2 months ago

Thank you @freak4pc for reporting this issue.

It looks that some auditing should be done indeed. Those are APIs I rarely use myself in this way, so I did not notice the problem.

I'll make an exhaustive list of methods that accept a generic DatabaseWriter or Reader.

I don't quite get why SE-0352 does not apply, though.

In the linked proposal text, search for "In the existing language, one could implement a shim..." The described technique will give you the needed workaround until GRDB7 can do what is needed so that no one faces the same issue as you do.

freak4pc commented 1 month ago

This section even more explicitly states you should be able to move from any to some and back, any chance this could be a bug?

https://github.com/apple/swift-evolution/blob/main/proposals/0352-implicit-open-existentials.md#moving-between-any-and-some

groue commented 1 month ago

This section even more explicitly states you should be able to move from any to some and back, any chance this could be a bug?

https://github.com/apple/swift-evolution/blob/main/proposals/0352-implicit-open-existentials.md#moving-between-any-and-some

I'll open a Github issue in the Swift repo if something does not behave as described by the proposal, yes. I'll need a closer look. Meanwhile, we just need GRDB users to be able to use any DatabaseWriter freely, since this is how we can use a pool in the app, and a queue in tests and previews. That's what's important.

(I'm late in my answer because I was in vacations 😎)

groue commented 1 month ago

Hello @freak4pc,

I listed all GRDB apis that accept a DatabaseWriter or DatabaseReader in a generic fashion (some DatabaseReader instead of any DatabaseReader), and gave them an existential. I got no compiler error, from Xcode 15.4 to Xcode 15.0.

In particular, this code compiles, which is as close as possible as your sample code:

func testValuesFromAnyDatabaseWriter(writer: any DatabaseWriter) {
    func observe<T>(
        fetch: @Sendable @escaping (GRDB.Database) throws -> T
    ) throws -> AsyncValueObservation<T> {
        let observation = ValueObservation.tracking(fetch)
        return observation.values(in: writer)
    }
}

All the GRDB demo apps use a common pattern, with a "manager" type that hides an any DatabaseWriter and exposes an any DatabaseReader:

public struct MyDatabaseManager {
    // Hidden because responsible of database integrity
    private let writer: DatabaseWriter

    // Public because reading can't harm
    public var reader: DatabaseReader { writer }
}

Those demo apps observe the database from this reader without any compiler error.

I'm not sure how you were able to get this compiler error.


EDIT: CI fails. I'll see what's wrong.

EDIT 2: Just a false positive timeout.

I'm closing this issue. There's no problem with DatabasePool/DatabaseQueue interchangeability via DatabaseWriter existential.

freak4pc commented 2 weeks ago

I missed this being closed, apologies. I'm still able to reproduce this on an entirely brand new project, example:

image

Attaching the project, i might be missing something: GRDBInterTest.zip

groue commented 2 weeks ago

Thank you, @freak4pc, for the sample project. Now everything is clear :-)

I recommend you open an issue at https://github.com/swiftlang/swift/, because the compiler does not handle implicitly unwrapped optionals (IUOs) correctly. It would not be reasonable to ask GRDB and all libraries that rely on SE-0352 to avoid generics just because all library users can use IUOs - the root problem lies in the compiler, which was delivered in an incomplete state.

I know two workarounds. The first, at call site:

// Temporary variable that fixes a compiler bug
let writer: DatabaseWriter = dbWriter
return observation.values(in: writer)

The other redefines the property:

private var dbWriter: DatabaseWriter { _dbWriter }
private var _dbWriter: DatabaseWriter!

func setup() {
    _dbWriter = ...
}

I might have to document one of those patterns, though. I'd favor the second.

groue commented 2 weeks ago

I recommend you open an issue at https://github.com/swiftlang/swift/

I filed https://github.com/swiftlang/swift/issues/74652

freak4pc commented 2 weeks ago

Was just about to file something, thank you for doing it 🙌 I'll follow there