Record.fetchOne(db) does not add "LIMIT 1" into sql query

[ealymbaev]

What did you do?

I did enable trace printing and found out that .fetchOne(db) query does not add "LIMIT 1" to raw sql statement

As long as I know there IS a difference for performance when you add limitation to query. FetchOne requires only one row.

Environment

GRDB flavor(s): GRDB GRDB version: 3.7 Installation method: CocoaPods Xcode version: Swift version: 4.2 Platform(s) running GRDB: iOS macOS version running Xcode:

[groue]

Hello @ealymbaev,

Thank you for your report. In my own personal usage of GRDB, I don't remember using fetchOne for queries which are not meant to return only one row. For example, when fetching by primary key. Or when fetching from a table which contains a single row.

Now, if you know actual use cases of fetchOne which means "the first in this set", then would you please share them here, so that we can discuss? There may be real needs for this extra LIMIT 1, after all.

[alextrob]

I've been caught out by this as well.

My use case was having a lot of data cached in a table and I wanted to query what the most recently cached item was. It looked something like this:

let latest = try dbQueue.read { db in
    CacheRecord.order([Column("cachedAt").desc]).fetchOne(db)
}

Adding .limit(1) to the query is an easy fix but easy to overlook.

[groue]

Hi @alextrob, thanks for your input, it's very valuable.

OK, we can look at a way to add an automatic LIMIT 1 to fetchOne. The impact of this change will not be huge, but you were able to convince me we can avoid surprises 👍

---

Currently, fetchOne in an extension method on the FetchRequest protocol. This protocol does not know which fetching method is called when it generates SQL statements, so it can not, in its current shape, deliver what we need:

protocol FetchRequest {
    // Called for fetchOne, fetchAll, fetchCursor
    func prepare(_ db: Database) throws -> (SelectStatement, RowAdapter?)
}

FetchRequest is the base protocol for custom requests as well as the two built-in requests QueryInterfaceRequest, and SQLRequest:

let request = SQLRequest<Player>(sql: "SELECT * FROM player ORDER BY score DESC") // SQLRequest<Player>
let request = Player.order(Column("score").desc) // QueryInterfaceRequest<Player>

I don't want SQLRequest to add LIMIT 1, because I prefer that GRDB avoids any kind of SQL parsing (in order to check if it should add a LIMIT clause or not). This is just too dangerous and difficult to implement correctly. And when users opt in for raw SQL, I'm not sure they expect the db library to fiddle with it.

But we may do something with QueryInterfaceRequest.

We have two options, as far as I can see:

1. Modify the FetchRequest protocol so that its prepare method is hinted at the kind of query it should generate. Update QueryInterfaceRequest so that it uses the new hint. Review each and every one occurrence of fetchOne (there are many) in order to check that it does provide the necessary hint.
2. Don't modify the FetchRequest protocol, but instead specialize fetchOne for the concrete type QueryInterfaceRequest.

I tend to think that option 1 should be preferred. (Edit: I'm positive it is the best. Option 2 must be avoided.)

Do you folks have any other idea or opinion? Would one of you eventually be interested in submitting a pull request with this new feature? Since GRDB 4 is on the way, we can introduce breaking changes. The moment is ideal, but we only have a few weeks.

[alextrob]

@groue Thanks for all that detail. I can have a go at implementing option 1 sometime in the next couple of days.

[ealymbaev]

I've been caught out by this as well.

My use case was having a lot of data cached in a table and I wanted to query what the most recently cached item was. It looked something like this:

let latest = try dbQueue.read { db in
    CacheRecord.order([Column("cachedAt").desc]).fetchOne(db)
}

Adding .limit(1) to the query is an easy fix but easy to overlook.

This is exactly the same case as mine. Hope this would be implemented in next releases. Meanwhile I will add .limit(1) as suggested.

[groue]

All right, @ealymbaev! I'm happy the .limit(1) technique lets you workaround this issue.

@alextrob this is great, thank you!

Since you are about to dive in, please let me give a comprehensive list of the various ways to call fetchOne, and provide a more detailed guidance. There are many fetchOne. All of them fit their own niche in application expressivity:

// FetchableRecord & TableRecord only
try Player.fetchOne()
try Player.fetchOne(key: 1)
try Player.fetchOne(key: ["email": "test@example.com"])

// FetchableRecord, DatabaseValueConvertible, StatementColumnConvertible, Row
try Player.fetchOne(db, sql: "SELECT ...")
let request = /* some FetchRequest whose RowDecoder associated type is Player */
try request.fetchOne(db)
try Player.fetchOne(db, request)
let statement = try db.makeSelectStatement(sql: "SELECT ...")
try Player.fetchOne(statement)

In the last examples, you see that not only record types are impacted. Row too, as well as values (DatabaseValueConvertible), and "fast values" (DatabaseValueConvertible & StatementColumnConvertible), can be fetched:

// All the following fetches should run `SELECT websiteURL FROM player LIMIT 1`:
let request = Player.select(Column("websiteURL"))

// FetchableRecord
try request.fetchOne(db)
try Player.fetchOne(db, request)

// Row
try Row.fetchOne(db, request)

// DatabaseValueConvertible
try URL.fetchOne(db, request)

// DatabaseValueConvertible & StatementColumnConvertible
try String.fetchOne(db, request)

Note that the two fetchOne(key:) variants do not need LIMIT 1, because they only work with primary keys and unique indexes. If we can avoid adding LIMIT 1 to these requests, I would appreciate. The quality of the generated SQL is also a metric I'm interested in.

fetchOne(_: Database, sql: String) and fetchOne(_: SelectStatement) are listed for the sake of exhaustivity, but they do not need extra LIMIT 1 either.

That's already enough information. The problem with progressive disclosure is that small changes can have a large impact, from beginner APIs to more advanced ones that only a few users happen to meet 😅

Please don't hesitate asking any question you may have!

[groue]

And oh, @alextrob, please target the GRDB-4.0 branch, because this is the current development branch!

[groue]

Superseded by #515