Auto-inject LIMIT 1 in bob.One#673
Conversation
|
This is an elegant solution... however, this means that running |
|
That is true. However even though I cannot think of a reason for not to include a LIMIT 1 on a single scan, we still have the I think this is a better default. |
|
I'll have to give this some thought. Generally, I prefer to avoid any unexpected behaviour. And while querying the full result set to retrieve just one is unexpected, it is a performance issue, not a "correctness" issue. Rather than merge this as-is, I'll have to first consider making queries immutable. There are a couple PRs for this, but that is a big change since everyone would have to update their use of the queries. |
|
I generally agree, but the thing is with this case that a lot of users, including myself at some point, thought that the old behaviour was unexpected. Since the majority of ORMs automatically apply a limit on single row retrieval it's what the regular user (and an AI coding assistant) will expect. Given there is plenty of opt-out with the I think the increased expectedness of how the implementation works, outweighs the concern of not altering the query behind the scene, ultimately providing a better developer experience. |
|
While I agree, my concern is about the effect of "misuse"
|
|
i would prefer to avoid this. my expectation for using |
|
I understand. As much as I like the idea of |
|
EntityFramework Core has this done pretty nicely.
So adding |
Oh!!! This could be a good direction in the meantime, to return an error if more than one row is returned. What do you think @jacobmolby @jay-babu |
|
I think the confusion here really comes from the original It's worth noting that EF Core, which is the reference point here, has no single-retrieval method that skips the limit like bob does. I think its semantically reversed to keep |
|
Okay. thanks for that. Here is my tentative plan:
Unfortunately, this means that it will be months before this can be properly addressed, but I am not at all comfortable rewriting queries underneath users. The alternative would be to fully clone the query before adding |
Closes #579.
bob.One(and the generated.One()methods that route through it) did not add aLIMITclause to the underlying query.Unless the caller explicitly applied an
sm.Limit(1)mod, the database would return every matching row andscan.Onewould silently discard all but the first — a quiet performance footgun on tables of any meaningful size.Changes
bob.Onenow callsSetLimitIfUnset(1)on the query before executing. If the query already has a row-limit configured, it is left alone.bob.Firstis added as the escape hatch: same semantics as the previousbob.One(no auto-limit). Use it when you genuinely want an unlimited query and only the first row scanned.bob.Limiterinterface introduced;BaseQueryforwards to its embedded expression, and each dialect'sSelectQueryimplementsSetLimitIfUnset— pickingLimit/Fetchfor plain selects andCombinedLimit/CombinedFetchforUNION/INTERSECT/EXCEPTqueries so the limit lands on the outer result.Scope
RETURNING) are unaffected.bob.QueryStmt.One) are unchanged — the SQL is baked atPreparetime.Migration
Most callers benefit automatically. Anyone relying on the previous unlimited-query behavior of
Oneshould switch those call sites tobob.First.