pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
10+ messages / 4 participants
[nested] [flat]
* BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
@ 2020-10-16 19:05 PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
0 siblings, 1 reply; 10+ messages in thread
From: PG Bug reporting form @ 2020-10-16 19:05 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: david@endpoint.com
The following bug has been logged on the website:
Bug reference: 16676
Logged by: David Christensen
Email address: david@endpoint.com
PostgreSQL version: 13.0
Operating system: OS X
Description:
Test case (found when demonstrating a queue mechanism for batches):
create table queue (id int generated always as identity, batch_id int not
null);
insert into queue (batch_id) values (1),(1),(2),(3);
postgres=# select * from queue;
id | batch_id
----+----------
1 | 1
2 | 1
3 | 2
4 | 3
(4 rows)
-- backend 1:
postgres=# begin;
BEGIN
postgres=*# select * from queue order by batch_id fetch first row with ties
for update skip locked;
id | batch_id
----+----------
1 | 1
2 | 1
(2 rows)
-- backend 2:
postgres=# select * from queue order by batch_id fetch first row with ties
for update skip locked;
id | batch_id
----+----------
4 | 3
(1 row)
-- QED
As you can see, the row id 3 with batch_id 2 is not returned as would be
implied by the strict ordering and the skipped locks. The working theory
here is the FETCH FIRST ROW WITH TIES locks the rows before deciding if they
should be included or not.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
@ 2020-10-19 22:02 ` David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 10+ messages in thread
From: David Christensen @ 2020-10-19 22:02 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
Proposed fix:
Reorder Limit/LockRows nodes to prevent locking extra tuples in FETCH FIRST WITH TIES
Reference bug report: 16676
1 file changed, 12 insertions(+), 12 deletions(-)
src/backend/optimizer/plan/planner.c | 24 ++++++++++++------------
modified src/backend/optimizer/plan/planner.c
@@ -2293,6 +2293,18 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
{
Path *path = (Path *) lfirst(lc);
+ /*
+ * If there is a LIMIT/OFFSET clause, add the LIMIT node.
+ */
+ if (limit_needed(parse))
+ {
+ path = (Path *) create_limit_path(root, final_rel, path,
+ parse->limitOffset,
+ parse->limitCount,
+ parse->limitOption,
+ offset_est, count_est);
+ }
+
/*
* If there is a FOR [KEY] UPDATE/SHARE clause, add the LockRows node.
* (Note: we intentionally test parse->rowMarks not root->rowMarks
@@ -2307,18 +2319,6 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
assign_special_exec_param(root));
}
- /*
- * If there is a LIMIT/OFFSET clause, add the LIMIT node.
- */
- if (limit_needed(parse))
- {
- path = (Path *) create_limit_path(root, final_rel, path,
- parse->limitOffset,
- parse->limitCount,
- parse->limitOption,
- offset_est, count_est);
- }
-
/*
* If this is an INSERT/UPDATE/DELETE, and we're not being called from
* inheritance_planner, add the ModifyTable node.
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
@ 2020-10-19 23:07 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2020-10-19 23:07 UTC (permalink / raw)
To: David Christensen <david@endpoint.com>; +Cc: pgsql-bugs@lists.postgresql.org
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?
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-10-19 23:59 ` David Christensen <david@endpoint.com>
2020-10-20 00:33 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
0 siblings, 1 reply; 10+ messages in thread
From: David Christensen @ 2020-10-19 23:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
> 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?
David
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
@ 2020-10-20 00:33 ` David Christensen <david@endpoint.com>
2020-10-20 13:41 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-10-21 19:12 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
0 siblings, 2 replies; 10+ messages in thread
From: David Christensen @ 2020-10-20 00:33 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
> 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.
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. )
Open to ideas on the appropriate fix.
David
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 00:33 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
@ 2020-10-20 13:41 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-10-20 14:16 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 10+ messages in thread
From: Alvaro Herrera @ 2020-10-20 13:41 UTC (permalink / raw)
To: David Christensen <david@endpoint.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org
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.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 00:33 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 13:41 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2020-10-20 14:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-10-20 14:23 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2020-10-20 14:16 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: David Christensen <david@endpoint.com>; pgsql-bugs@lists.postgresql.org
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2020-Oct-19, David Christensen wrote:
>> 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.
Um, why not? I don't have a position yet on whether this is a good way
to fix it; but if we did do it, AFAICS the only thing we'd have to be
careful of in v13 is not renumbering existing NodeTag values.
If your concern is just that EXPLAIN plans will look different, the same
could be said of David's other proposal.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 00:33 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 13:41 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-10-20 14:16 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-10-20 14:23 ` David Christensen <david@endpoint.com>
2020-10-20 16:18 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
0 siblings, 1 reply; 10+ messages in thread
From: David Christensen @ 2020-10-20 14:23 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-bugs@lists.postgresql.org
> On Oct 20, 2020, at 9:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> On 2020-Oct-19, David Christensen wrote:
>>> 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.
>
> Um, why not? I don't have a position yet on whether this is a good way
> to fix it; but if we did do it, AFAICS the only thing we'd have to be
> careful of in v13 is not renumbering existing NodeTag values.
>
> If your concern is just that EXPLAIN plans will look different, the same
> could be said of David's other proposal.
If we can determine the scope to which these different nodes might be used and make it so it still uses the existing node structure for most existing cases (i.e., maybe an additional parameter to create_lockrows_plan or similar) then that might work; AFAIK, this is the first time we’ve had to consider this sort of thing (though I do wonder if there might be something in window functions which might bump into this sort of issue).
I’m working on an isolation test to formalize this failure, which I can do as a separate patch if we want to have this prior to implementing a fix.
--
David Christensen
Senior Software and Database Engineer
End Point Corporation
david@endpoint.com
785-727-1171
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 00:33 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 13:41 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-10-20 14:16 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-20 14:23 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
@ 2020-10-20 16:18 ` David Christensen <david@endpoint.com>
0 siblings, 0 replies; 10+ messages in thread
From: David Christensen @ 2020-10-20 16:18 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-bugs@lists.postgresql.org
> On Oct 20, 2020, at 9:23 AM, David Christensen <david@endpoint.com> wrote:
>
>> On Oct 20, 2020, at 9:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>>> On 2020-Oct-19, David Christensen wrote:
>>>> 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.
>>
>> Um, why not? I don't have a position yet on whether this is a good way
>> to fix it; but if we did do it, AFAICS the only thing we'd have to be
>> careful of in v13 is not renumbering existing NodeTag values.
>>
>> If your concern is just that EXPLAIN plans will look different, the same
>> could be said of David's other proposal.
>
> If we can determine the scope to which these different nodes might be used and make it so it still uses the existing node structure for most existing cases (i.e., maybe an additional parameter to create_lockrows_plan or similar) then that might work; AFAIK, this is the first time we’ve had to consider this sort of thing (though I do wonder if there might be something in window functions which might bump into this sort of issue).
>
> I’m working on an isolation test to formalize this failure, which I can do as a separate patch if we want to have this prior to implementing a fix.
Enclosed is the (currently failing) isolation test for the expected behavior.
--
David Christensen
Senior Software and Database Engineer
End Point Corporation
david@endpoint.com
785-727-1171
Attachments:
0001-Add-isolation-test-for-FETCH-FIRST-ROW-WITH-TIES-FOR.patch (application/octet-stream, 2.7K) download
From ce874d9dd8afee20e5bd547b9cc17d1803168aa0 Mon Sep 17 00:00:00 2001
From: David Christensen <david@endpoint.com>
Date: Tue, 20 Oct 2020 10:17:58 -0500
Subject: [PATCH] Add isolation test for FETCH FIRST ROW WITH TIES FOR UPDATE
SKIP LOCKED
---
src/test/isolation/expected/skip-locked-5.out | 14 +++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/skip-locked-5.spec | 29 +++++++++++++++++++
3 files changed, 44 insertions(+)
create mode 100644 src/test/isolation/expected/skip-locked-5.out
create mode 100644 src/test/isolation/specs/skip-locked-5.spec
diff --git a/src/test/isolation/expected/skip-locked-5.out b/src/test/isolation/expected/skip-locked-5.out
new file mode 100644
index 0000000000..87fc2d8aa1
--- /dev/null
+++ b/src/test/isolation/expected/skip-locked-5.out
@@ -0,0 +1,14 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1a s2a s1b s2b
+step s1a: SELECT * FROM foo ORDER BY batch FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED;
+id batch
+
+1 1
+2 1
+step s2a: SELECT * FROM foo ORDER BY batch FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED;
+id batch
+
+3 2
+step s1b: COMMIT;
+step s2b: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index aa386ab1a2..9e32b00149 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -64,6 +64,7 @@ test: skip-locked
test: skip-locked-2
test: skip-locked-3
test: skip-locked-4
+test: skip-locked-5
test: drop-index-concurrently-1
test: multiple-cic
test: alter-table-1
diff --git a/src/test/isolation/specs/skip-locked-5.spec b/src/test/isolation/specs/skip-locked-5.spec
new file mode 100644
index 0000000000..83bb3de00f
--- /dev/null
+++ b/src/test/isolation/specs/skip-locked-5.spec
@@ -0,0 +1,29 @@
+# Test SKIP LOCKED with FETCH FIRST ROW WITH TIES
+
+setup
+{
+ CREATE TABLE foo (
+ id int PRIMARY KEY,
+ batch int NOT NULL
+ );
+ INSERT INTO foo VALUES (1, 1), (2, 1), (3, 2), (4, 3);
+}
+
+teardown
+{
+ DROP TABLE foo;
+}
+
+session "s1"
+setup { BEGIN; }
+step "s1a" { SELECT * FROM foo ORDER BY batch FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED; }
+step "s1b" { COMMIT; }
+
+session "s2"
+setup { BEGIN; }
+step "s2a" { SELECT * FROM foo ORDER BY batch FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED; }
+step "s2b" { COMMIT; }
+
+# s1 returns the two rows associated with batch 1 and takes the lock; s2 should skip those two rows due to being locked and should return the batch 2 row.
+permutation "s1a" "s2a" "s1b" "s2b"
+
--
2.24.3 (Apple Git-128)
=
* Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
2020-10-20 00:33 ` Re: BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return David Christensen <david@endpoint.com>
@ 2020-10-21 19:12 ` David Christensen <david@endpoint.com>
1 sibling, 0 replies; 10+ messages in thread
From: David Christensen @ 2020-10-21 19:12 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-bugs@lists.postgresql.org
> On Oct 19, 2020, at 7:33 PM, David Christensen <david@endpoint.com> wrote:
>
> 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. )
This does appear to be quite a bit harder than originally hoped for on the surface; creating a node to “pass-thru” unlocked tuples without locking them would require building out some additional infrastructure that does not appear to be present in all AMs; something to allow a test for locks without actually calling table_tuple_lock() formally. Perhaps adding a param to indicate “test” only—not sure if something like similar to heapam's “test_lockmode_for_conflict()” is something we’d need to had. (Or even a new lock mode which doesn’t create RowMarks but skips any conflicting modes.)
That said, I think it might be worth trying to go down the road of “forbid this behavior”, particularly if a workaround is available. Perhaps looking into why Álvaro’s plan was generating the multiple LockRows nodes would be more fruitful and less likely to cause backpatching pain.
David
--
David Christensen
Senior Software and Database Engineer
End Point Corporation
david@endpoint.com
785-727-1171
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2020-10-21 19:12 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16 19:05 BUG #16676: SELECT ... FETCH FIRST ROW WITH TIES FOR UPDATE SKIP LOCKED locks rows it doesn't return PG Bug reporting form <noreply@postgresql.org>
2020-10-19 22:02 ` David Christensen <david@endpoint.com>
2020-10-19 23:07 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-10-19 23:59 ` David Christensen <david@endpoint.com>
2020-10-20 00:33 ` David Christensen <david@endpoint.com>
2020-10-20 13:41 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2020-10-20 14:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-10-20 14:23 ` David Christensen <david@endpoint.com>
2020-10-20 16:18 ` David Christensen <david@endpoint.com>
2020-10-21 19:12 ` David Christensen <david@endpoint.com>
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