Grdb.swift: Crash using more than one DatabaseCollation

[benasher44]

What did you do?

Attempt to use a DatabaseCollation after installing multiple.

What did you expect to happen?

Using a custom collation successfully calls the collation function.

What happened instead?

We see a bad access crash

Environment

GRDB flavor(s): GRDB (linking custom SQLite)
GRDB version: 4.1.1
Installation method: CocoaPods
Xcode version: 10.2.1
Swift version: 5
Platform(s) running GRDB: iOS
macOS version running Xcode: 10.14.5

Demo Project

No demo project, but here is what the code looks like:

func installCollation(_ pool: DatabasePool) {
    let standardCollation = DatabaseCollation("STANDARD") {
        $0.localizedStandardCompare($1)
    }
    let unicodeCollation = DatabaseCollation("UNICODE") {
        $0.localizedCaseInsensitiveCompare($1)
    }
    pool.add(collation: standardCollation)
    pool.add(collation: unicodeCollation)
}

Investigative Notes

After the second call to add(collation:, DatabasePool's internal collation Set only has one element. I would expect it to have 2. I believe this is because DatabaseCollation's hash function always hashes to 0, which results in every call to add ejecting the previous collation from the set. If you then go to use one of the collations, which has been ejected, it will crash attempting to use the collation function from previously-ejected DatabaseCollation instances, as they have been deallocated (no longer retained by the Set).

I can workaround this by storing these collations in globals (ensuring the instances remain valid for the lifetime of the app):

private let standardCollation = DatabaseCollation("STANDARD") {
    $0.localizedStandardCompare($1)
}

private let unicodeCollation = DatabaseCollation("UNICODE") {
    $0.localizedCaseInsensitiveCompare($1)
}

func installCollation(_ pool: DatabasePool) {
    pool.add(collation: standardCollation)
    pool.add(collation: unicodeCollation)
}

[benasher44]

This has further problems for connections in the pool that haven't been created yet. New connections won't have access to the original collations (for the DatabasePool to add them) since they're not in the Set any longer.

[groue]

Hello @benasher44, thanks for the detailed bug report!

Until it is fixed, here is the workaround: GRDB already ships with those collations. They are so frequently needed that you don't need to add them: they are available by default.

// Table creation
try db.create(table: "player") { t in
    // Sort names in a localized case insensitive way
    t.column("name", .text).collate(.localizedCaseInsensitiveCompare)
    ...
}

// Sort files like the Finder:
let files = try File
    .order(Column("name").collating(.localizedStandardCompare))
    .fetchAll(db)

Those are mentionned in the String Comparison chapter.

Not mentioned in the main README: beware if you use those locale-dependent collations in table indexes. You'll then have to instruct SQLite to rebuild those indexes after a locale change:

// After a locale change, rebuild indexes as in https://www.sqlite.org/lang_reindex.html
try dbQueue.write { db in
    try db.reindex(collation: .localizedStandardCompare)
    try db.reindex(collation: .localizedCaseInsensitiveCompare)
}

I'll be off for a few weeks. I hope you won't need more collations until I'm back 😅

[groue]

The tests that have to be updated: https://github.com/groue/GRDB.swift/blob/v4.1.1/Tests/GRDBTests/DatabasePoolCollationTests.swift

[benasher44]

I was able to further workaround this by using Configuration.prepareDatabase and reverting back to the sqlite3 C API for installing collations.

[benasher44]

Thanks!

[benasher44]

No worries! I appreciate the workaround info. We unfortunately have column definitions that use the name STANDARD, so I don't think I can use the built-in ones (at least not without writing some migrations). But, it's great to know those are available! Since we have a workaround in place, I can just lean on that, and I look forward to removing our C-based code, once you have a chance to fix! Thanks again!

[groue]

That sounds quite reasonable. Thanks for your patience, and for putting your SQLite skills to the rescue!

[groue]

Fixed in GRDB 4.2.0