From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: David Christensen <david@endpoint.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
Date: Tue, 20 Oct 2020 10:41:14 -0300
Message-ID: <20201020134114.GA11045@alvherre.pgsql> (raw)
In-Reply-To: <7EA65D44-7C3A-4FC2-8DE1-1F59BBB10B37@endpoint.com>
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.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: alvherre@alvh.no-ip.org, david@endpoint.com, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
In-Reply-To: <20201020134114.GA11045@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox