fatal error: unexpectedly found nil while unwrapping an Optional value - inserting a row with .Ignore

[dta256]

I insert a row into a db table like this:

    try database.run(keywords.insert(or: .Ignore,
        self.keyword <- keyword,
        self.kwpath <- path
        ))

if insert fails the app crashes since it tries to unwrap lastInsertRowid in the last line of the function below (in SQLite/Typed/Query.swift file class Connection lines 964-970), which is nil:

public func run(query: Insert) throws -> Int64 {
    let expression = query.expression
    return try sync {
        try self.run(expression.template, expression.bindings)
        return self.lastInsertRowid!
    }
}

The lastInsertRowid is nil because insert has failed, and we reach the line where we try to unwrap lastInsertRowid because I have specified "or: .Ignore" when calling insert.

[dta256]

The run function for Insert query should return an Optional from my point of view, then unwrapping is not needed and everyone is happy.

[stephencelis]

This is a dupe of #235 and #295. Would love a proper fix for it, but haven't had time to attack it.

[dta256]

Sorry, I looked through the list but must have missed #235. However I would oppose #295 - it misuses the concept of optionals. Instead of returning nil it returns -1 if lastInsertRowid does not exist. That is the returned value still has to be tested if it is a valid rowid, but less intuitive.
For my understanding, why have you rejected #237? Would you prefer to get unwrapped rowid for normal inserts and have an optional for "insert(or:" case?

[stephencelis]

I agree with your take on #295 (which is why it wasn't merged in). #237 fell off my radar. It wasn't explicitly rejected. GitHub unfortunately auto-closes PRs that are made against branches that get deleted. In this case, that PR was made against a short-lived branch. If you'd like to reopen such a pull and address my one comment, I'd be happy to merge it in!

My take: the main insert() method shouldn't return an optional. It could however, either/both:

1. throw a catchable error for failed ignores (this would allow try? db.insert(or: .Ignore, …) to work)
2. overload insert() with an optional version for .Ignore only: insertOrIgnore().

[dta256]

Yes, I agree, having both alternatives would be the best solution. Ok, I will try to come up with a solution.

[p2]

Any progress on solving this? This breaks the library if one wants to use INSERT OR IGNORE.

[jberkel]

should be fixed in 0.11.1 / #532