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@endpoint.com> wrote:
> > 
> >> On Oct 19, 2020, at 6:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> 
> >> David Christensen <david@endpoint.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.