Hmm, it seems like the subquery is getting re-run on every single row of the task queue table, which seems like a performance issue in addition to correctness.
Personally, if I were going to put any part of that query in a CTE, it would have been (just) the select, as I generally prefer CTE's to subqueries.
I'm really not sure what motivated the author to create the CTE in the first place, as the final select seems to be essentially the identity function.
I don’t know about “gotcha”. This sounds like a full blown planner bug to me.
Why was the author not just doing returning *
No need for the CTE to select everything…
DELETE FROM task_queue
WHERE id = ( -- Use '=' for a single expected ID
SELECT id FROM task_queue
WHERE queue_group_id = 15
LIMIT 1
FOR UPDATE SKIP LOCKED
)
RETURNING item_id;
I don't understand where that item_id value comes from, since that's not a column that's mentioned anywhere else in the query.I guess it must be an unmentioned column on that task_queue table?
This is interesting - I don’t expect undefined behavior like this. Should the query delete more than one row or not?
The blog post doesn’t really give answers - it isn’t obvious to me that the second query can’t be executed in the exact same way. The even cop to this fact:
> This structure appears less ambiguous to the planner and typically encourages it to evaluate the subquery first.
So then - their bug still exists maybe?
I have thoughts - probably we do expect it never should delete multiple rows, but after considering the query plan, I can see it being ambiguous. But I would have expected Postgres to use only a single interpretation.
Interesting to think about how to guard against these cases where query optimisation leads to unexpected results.
I mean this could lead to serious bugs. What can be a way to detect these using linters or in CI before they hit the production.
DELETE in SKIP LOCKED scenario is recipe for failure. God/Postgres gave you transactions for atomicity... only UPDATE w/ SKIP LOCKED, holy brother in Christ, and only DELETE completed normally.
Since I don't often write raw SQL, I can only assume the author named their CTE `deleted_tasks` to elucidate that the query might delete multiple items. Otherwise, it makes little sense, for they intended to "pop" a single row, and yet their aptly named `deleted_tasks` ended up removing more than one!
The query reads to me like a conceptual mish-mash. Without understanding what the innermost `SELECT` was meant to accomplish, I'd naturally interpret the `WHERE id IN (...)` as operating on a set. But the most sacrilegious aspect is the inclusion of `FOR UPDATE SKIP LOCKED`. It assumes a very specific execution order that the query syntax doesn't actually enforce.
Am I right to think that not avoiding lock contention, i.e. omitting `SKIP LOCKED` would have actually produced the intended result?