Support for INSERT OR REPLACE (was: Save method for RTree and FTS)

[hdlj]

Hello Groue!

GRDB crashes on the save() method when it is applied to a RTree table or a FTS table because the primary key is hidden.

The workaround, I used, is this sql statement

INSERT OR REPLACE INTO ... VALUES(...)

Do you think it is possible to have a specific behaviour when the primary key is hidden for the save() method ?

Thanks

[groue]

@hdlj, I'm not familiar enough with RTree and FTS. I'd be happy with a sample code which exhibits the bug (and would be fixed by the INSERT OR REPLACE).

[hdlj]

Thanks for your reply ! :)

Here some code:

import GRDB


var configuration = Configuration()
configuration.trace = { print($0) }
let dbQueue = DatabaseQueue(configuration: configuration)   // Memory database


var migrator = DatabaseMigrator()
migrator.registerMigration("createRtree") { db in
    try! db.execute("CREATE VIRTUAL TABLE rtree_table USING rtree(id, min_x, max_x)")
}
try! migrator.migrate(dbQueue)

final class RTree1DRow: Record {
    let id: Int64
    let maxX: Double
    let minX: Double

    init(id: Int64, minX: Double, maxX: Double) {
        self.id = id
        self.maxX = maxX
        self.minX = minX
        super.init()
    }

    required init(row: Row) {
        id = row.value(named: "id")
        maxX = row.value(named: "max_x")
        minX = row.value(named: "min_x")
        super.init(row: row)
    }

    override var persistentDictionary: [String : DatabaseValueConvertible?] {
        return [
            "id": id,
            "max_x": maxX,
            "min_x": minX,
        ]
    }

    override static var databaseTableName: String { return "rtree_table" }
}

let row = RTree1DRow(id: 1, minX: 1.01, maxX: 1.02)


// Save twice will raise an error
do {
    try dbQueue.write { db in
        try row.save(db)
        try row.save(db)
    }
} catch {
    print("Could not be saved twice")
}

do {
    try dbQueue.write { db in
        try db.execute("INSERT OR REPLACE INTO rtree_table (id, min_x, max_x) VALUES(?,?,?)", arguments: StatementArguments([1, 1.01, 1.02]))
        try db.execute("INSERT OR REPLACE INTO rtree_table (id, min_x, max_x) VALUES(?,?,?)", arguments: StatementArguments([1, 1.01, 1.02]))
    }
} catch {
    print("Should not raise an error")
}

[groue]

Thanks @hdlj, I'll take a look.

[groue]

Thanks, the sample code behaves as you say.

Virtual tables are not normal tables, and behaves in any odd way they want. Your own issue #80 has revealed that selecting a FTS virtual table yields database modification events (?!). Now rtree tables reveal funny as well.

I'm not sure GRDB can automatically deal with all those odd cases.

Yet it can help. In your specific case, it looks like the three methods save, update and insert should all perform an INSERT OR REPLACE. Am I wrong?

If so, we may consider an option to MutablePersistable and Persistable:

protocol MutablePersistable {
    static var replaceOnConflict: Bool
}

final class RTree1DRow: Record {
    override class var replaceOnConflict: Bool { return true }
}

I'll have to look for more use cases around INSERT OR REPLACE in order to make sure this solution is general enough - and write documentation.

What do you think?

Note to myself: if this flag is true, the method didInsert(with:forColumn) must not be called. That's because SQLite won't tell if INSERT OR REPLACE did insert a row or not, and it's impossible to reliably get the rowID of the inserted-or-replaced row.

[groue]

The current work in progress adds replaceOnInsertionConflict to the MutablePersistable protocol. If true, it has GRDB generate INSERT OR REPLACE instead of INSERT. Precisely speaking:

• insert() always generates INSERT OR REPLACE
• update() is unchanged: it still generates UPDATE, and crashes in record has no primary key (your case)
• save() is unchanged: it still falls back to insert when record has no primary key or nil primary key, and thus generates INSERT OR REPLACE in your case.

[hdlj]

Great! thanks! this approach sounds good to me!

Just a small proposal, if no primary key is found and the insert fails, I could be great to log an error like "Did you see this parameterreplaceOnInsertionConflict?" in order to make the feature discoverable , what do you think ? maybe it is overkill

[groue]

Just a small proposal, if no primary key is found and the insert fails, I could be great to log an error like "Did you see this parameterreplaceOnInsertionConflict?" in order to make the feature discoverable , what do you think ? maybe it is overkill

I expect most insert failures to reveal an application bug (unintended violation of a unique index or relational constraint). Pushing INSERT OR REPLACE would both foster hiding bugs under the carpet, and potentially introduce new bugs (INSERT OR REPLACE replaces, which means that it deletes before inserting - this triggers deletion cascade in foreign tables).

My little experience makes me feel that conflict handling should generally be handled at the table definition: If one generally performs INSERTs, and an occasional INSERT OR REPLACE, it looks like a code smell to me (at least something that deserves a comment). Rtree and fts tables can't specify conflict handling at the table level, unfortunately. The replaceOnInsertionConflict parameter looks good to me so far: it both handles rtree/fts, and has the API foster global ON REPLACE (either in the table definition, either at the type level).

Note to myself: reuse SQLConflictResolution instead of a Boolean

[groue]

Since both INSERT and UPDATE allow conflict handling, let's introduce MutablePersistable.persistenceConflictPolicy!

The feature is ready, for Swift 3 only, on the Issue118 branch.

[hdlj]

Awesome! Thanks!

[groue]

@hdlj Support for INSERT OR REPLACE has shipped in v0.84.0 (documentation).