ValueObservation doesn't trigger for fetchCount

[steipete]

What did you do?

I use this code to get data via a value observer:

    public struct Statistics: Equatable {
        public let tweetCount: Int
    }

    /// Return database infos
    public func statisticsPublisher() -> AnyPublisher<Statistics, Error> {
        ValueObservation.tracking { db in
			// If I uncomment this, the observer works.
            // _ = try Tweet.all().fetchAll(db)
            return Statistics(tweetCount: try Tweet.all().fetchCount(db))
        }
        //.removeDuplicates()
        .publisher(in: dbPool).eraseToAnyPublisher()
    }

Is this a known limitation?

When I call print() the difference is:

tracked region: tweet(*)

vs

tracked region: Tweet(lotsOfProperties,text,updatedAt,userId),tweet(*)

What did you expect to happen?

The observer should fire as new rows are added.

What happened instead?

The observer isn't detecting changes.

Environment

GRDB 5.6, iOS 14.5, Xcode 12.5b3, macOS Big Sur 11.3b6

[groue]

Hello @steipete,

Observing fetchCount is supposed to work, and tested. I can't reproduce:

Sample code

import PlaygroundSupport
import Combine
import Foundation
import GRDB

PlaygroundPage.current.needsIndefiniteExecution = true

// Database setup
let path = NSTemporaryDirectory().appending("test.sqlite")
let dbPool = try DatabasePool(path: path)
try dbPool.write { db in
    try db.execute(sql: "DROP TABLE IF EXISTS tweet")
    try db.create(table: "tweet") { t in
        t.autoIncrementedPrimaryKey("id")
    }
}

// Minimal models
struct Tweet: TableRecord { }
struct Statistics {
    var tweetCount: Int
}

// The publisher
let publisher = ValueObservation.tracking { db in
    return Statistics(tweetCount: try Tweet.all().fetchCount(db))
}
.publisher(in: dbPool).eraseToAnyPublisher()

// Ignition!
let cancellable = publisher.sink(receiveCompletion: { _ in }, receiveValue: { statistics in
    print("fresh statistics: \(statistics)")
})

// Expect change notification
DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
    try! dbPool.write { db in
        try db.execute(sql: """
            INSERT INTO tweet DEFAULT VALUES;
            INSERT INTO tweet DEFAULT VALUES;
            """)
    }
    print("Will we count two tweets?")
}

Output (the initial duplicate value is due to #937):

fresh statistics: Statistics(tweetCount: 0)
fresh statistics: Statistics(tweetCount: 0)
Will we count two tweets?
fresh statistics: Statistics(tweetCount: 2)

I tried adding columns, giving Tweet properties and make it fetchable, naming its table Tweet (capitalized), etc, but I could not reproduce the change blindness.

Yet we may be onto something. The Tweet(lotsOfProperties,text,updatedAt,userId),tweet(*) region is unexpected. Database name canonicalization did not happen so that both regions are merged into a single one tweet(*) or Tweet(*) (the actual name of the table in the database). This may explain why some changes reported by sqlite aren't matched and reported.

Let's figure out! Can you please give more information about the table for tweets, and the way you define the Tweet record?

[groue]

OK @steipete, I have tests that fail due to table name case-sensitivity.

A workaround is to make sure GRDB only builds SQL requests that use the exact same database table name as in the database file:

struct Tweet {
    #warning("TODO: remove when GRDB#954 is fixed")
    static let databaseTableName = "TwEeT"
}

This should do the trick. Thanks for the report! A fix will ship shortly!

[steipete]

Dang you are fast! Do you recommend to keep all table names lowercased? I current use CamelCase for the tables.

[groue]

Dang you are fast! Do you recommend to keep all table names lowercased? I current use CamelCase for the tables.

SQLite is case-insensitive: GRDB is supposed to accept all casing flavors and variations, and this is why this issue reveals a bug, and that the test suite is incomplete :-)

The documentation makes a recommendation, which is to name database tables just as a swiftIdentifier: english, camel-cased, singular, and lowercase first letter. This recommendation is over-specified, and not following it is possible:

• English: helps associations derive identifiers that work out-of-the-box ("child" -> "children"). Not following this convention is OK, but one has to provide explicit strings in some places.
• Camel-cased and singular: because the default table name of record types is derived from their name: "PlayerAward" -> "playerAward". Not following this convention is OK (using underscores, whatever), but one has to provide an explicit table name for each record type.
• Lowercase first letter: it just happens that it matches my personal convention, and that some recommendation had to be written (some users don't know that SQLite is case-insensitive, and/or like having a frame). Not following this convention is not supposed to have any consequence (this issue has revealed that it has 😬)

[steipete]

Thanks for the explanation! I've changed my tables to your default, but good to see this fixed!

[groue]

Fixed in v5.7.4