perf: Avoid hitting the database when queries can be statically analyzed as never returning any results - #4780
Conversation
…zed as never returning any results
I often find myself writing code like this:
```
def get_users_by_ids(ids)
def get_users_by_ids([]), do: []
def get_users_by_ids(ids) do
from(u in User, where: u.id in ^ids)
|> Repo.all()
end
```
Today, if I left off that first function head, Ecto would still hit the database with a query like `SELECT * FROM users WHERE false`. Obviously that's not a heavy query, but to the extent that I can avoid putting trivial load on my database (not to mention the round-trip query time), I would prefer to do so.
To that end, this PR adds a gate to `Ecto.Repo.Queryable`'s `stream/3`, `aggregate/{4,5}`, and `execute/4` functions to first check whether the WHERE clauses can be statically shown to be unsatisfiable, and if so, return the appropriate empty result.
(Note that the diff looks quite large, but the delta to those existing functions is largely whitespace.)
66736e1 to
04b5e9a
Compare
|
Thanks @s3cur3! I am not sure we should go down this road because it only works from time to time. The smarter we are, the more surprising it becomes to understand when it works and when it doesn't. Perhaps we could support straight-forward patterns, like |
|
Can you elaborate on "it only works from time to time"? I guess to me it feels a little like a compiler optimization... one of those things that's not guaranteed, but to the extent that the compiler can help you out, it's nice when it does. Re: joining with an |
|
per your own description, it is something you are actively aiming for, so we all stand to gain by making it clearer to understand when it happens vs hoping the compiler does it. It is also easier to maintain. |
|
FWIW, rather then not executing the queries silently, I'd consider logging a warning as more useful and potentially actionable signal. Just saying. |
|
I think what José is trying to get at is, if we go down this road we end up taking the job of the database query planner and execution. That can be a nontrivial amount of complexity as different implementation might diverge from your initial expectations, especially given join on clauses and wheres. You could imagine some queries returning results, but with this they wouldn’t as it never gets to the database. |
|
Okay, fair enough. We'll put it in the app layer. |
4 participants
I often find myself writing code like this:
Today, if I left off that first function head, Ecto would still hit the database with a query like
SELECT * FROM users WHERE false. Obviously that's not a heavy query, but to the extent that I can avoid putting trivial load on my database (not to mention the round-trip query time), I would prefer to do so. Furthermore, it would be wonderful if Ecto would do this short-circuiting for me, so that I don't need to add those function heads to check for empty queries myself.To that end, this PR adds a gate to
Ecto.Repo.Queryable'sstream/3,aggregate/{4,5}, andexecute/4functions to first check whether the WHERE clauses can be statically shown to be unsatisfiable, and if so, return the appropriate empty result.(Note that the diff looks quite large, but the delta to those existing functions is largely whitespace.)