Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return

On 2020-Oct-19, David Christensen wrote:

> > On Oct 19, 2020, at 6:59 PM, David Christensen <david(at)endpoint(dot)com> wrote:
> >
> >> On Oct 19, 2020, at 6:07 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>
> >> David Christensen <david(at)endpoint(dot)com> writes:
> >>> Proposed fix:
> >>> Reorder Limit/LockRows nodes to prevent locking extra tuples in FETCH FIRST WITH TIES
> >>
> >> Isn't that going to break more cases than it fixes?
> >
> > In the case of Limit, isn’t LockRows supposed to only lock the number of actual rows returned?
> >
> > What are the scenarios that this might break and do you have any ideas for alternate fixes?
>
> Will now that I think about it, if the LockRows node is responsible
> for skipping locked rows, etc then my proposed solution is clearly
> wrong.

I tried your proposed patch yesterday and found that the second query
returns no rows rather than two rows, which is not an improvement.

> Maybe splitting LockRows into two nodes, one for locking and one for
> emitting unlocked nodes then interleaving Limit in between? (Or only
> doing something along these lines for this admittedly narrow use case.)

I was having a similar idea, but the main reason I don't think it's a
good fix is that we can't backpatch such a change to pg13.

I was thinking if it was possible to modify Limit so that it would
"peek" into its child node without "actually reading". But that's not
possible in our executor implementation AFAIK -- you only have
ExecProcNode as an interface, there's no way to affect what happens
underneath.

Maybe an approach is to forbid a FOR UPDATE clause in a query that has a
LIMIT WITH TIES clause; so we'd force the user to write a subquery for
LIMIT WITH TIES and then apply FOR UPDATE to an outer query. However,
I'm surprised to find that we have *two* LockRows nodes when doing that:

explain select * from (select * from queue order by batch_id fetch first 2 rows with ties) t for update skip locked;
QUERY PLAN
────────────────────────────────────────────────────────────────────────────────────────
LockRows (cost=55.20..55.27 rows=2 width=40)
-> Subquery Scan on t (cost=55.20..55.25 rows=2 width=40)
-> Limit (cost=55.20..55.23 rows=2 width=14)
-> LockRows (cost=55.20..83.45 rows=2260 width=14)
-> Sort (cost=55.20..60.85 rows=2260 width=14)
Sort Key: queue.batch_id
-> Seq Scan on queue (cost=0.00..32.60 rows=2260 width=14)
(7 filas)

and of course, we still have the behavior where the third row is
skipped.