Should we use DatabaseWriter for reading from database?

[simensol]

In World.swift of the GRDBCombineDemo, only DatabaseWriter is tied to self.database:

import GRDB

/// Dependency Injection based on the "How to Control the World" article:
/// https://www.pointfree.co/blog/posts/21-how-to-control-the-world
struct World {
    /// Access to the players database
    func players() -> Players { Players(database: database()) }

    /// The database, private so that only high-level operations exposed by
    /// `players` are available to the rest of the application.
    private var database: () -> DatabaseWriter

    /// Creates a World with a database
    init(database: @escaping () -> DatabaseWriter) {
        self.database = database
    }
}

var Current = World(database: { fatalError("Database is uninitialized") })

Then, in for example the refresh() function in Players.swift, database.write is used to read from the database:

    func refresh() throws {
        try database.write { db in
            if try Player.fetchCount(db) == 0 {
                // Insert new random players
                [...]
    }

Can database.write be used to read from the database? According to the GRDB documentation, the correct way of reading from the database is by using database.read, like:

// Read values:
try dbPool.read { db in
    let places = try Place.fetchAll(db)
    let placeCount = try Place.fetchCount(db)
}

So my question is: Is it good practice to read from the database using database.write? And if not, shouldn't the World.swift of the GRDBCombineDemo also implement DatabaseReader?

[groue]

Hello @simensol,

Is it good practice to read from the database using database.write?

When you only read, it is recommended to prefer read over write. The intent is more clear in your code, and DatabasePool can optimize database accesses.

When you read AND write, then the rule 2 of the Concurrency Guide kicks in. Let's call it the "Isolation rule". It states:

Group related statements within a single call to a DatabaseQueue or DatabasePool database access method.

Let's see how the rule applies to the sample code you quoted above. It does not only read. It also writes, and the writes actually depend on the fetched values. Those are totally related, and the rule 2 fully applies.

See how it fills the database with random players if and only if the database is empty:

try database.write { db in
    if try Player.fetchCount(db) == 0 {
        // Insert new random players
        for _ in 0..<8 {
            var player = Player(id: nil, name: Player.randomName(), score: Player.randomScore())
            try player.insert(db)
        }
    } else {
        ...
    }
}

We must use write, here. Grouping in the same database access method the code that checks for the number of players, and the code that insert fresh players indeed prevents bugs.

Let's consider how to do it in the wrong way:

// DON'T DO THAT
let playerCount = try database.read { db in
     try Player.fetchCount(db)
}

try database.write { db in
    if try playerCount == 0 {
        // Insert new random players
        for _ in 0..<8 {
            var player = Player(id: nil, name: Player.randomName(), score: Player.randomScore())
            try player.insert(db)
        }
    } else {
        ...
    }
}

The code above is not thread-safe. Between the read, and the write, anything can happen. Some other thread may modify the database in between. You may end up inserting 8 new players in a non-empty database, when this is not your intent. This is called a bug.

And even if no other thread does sneaks in today, your application will evolve, and some other thread may modify the database right at the wrong time, eventually.

The only way to write rock-solid code that will never break is to group related statements within a single call to a DatabaseQueue or DatabasePool database access method.

[simensol]

Thanks for a great explanation, @groue! I will make my code thread-safe from now on.

However, one thing is still unclear to me: Can we do DatabaseWriter.read? My intuition is that we should do DatabaseReader.read instead. But in World.swift of the GRDBCombineDemo, only DatabaseWriter is tied to self.database.

[groue]

That's great, @simensol :-)

DatabaseWriter is a sub-protocol of DatabaseReader: when you can write, you can read (but the opposite is not true). The World object of the demo app uses a DatabaseWriter, because the app needs to write in the database!

[simensol]

I see, @groue! Thanks again :)

[groue]

Happy GRDB!