Skip to content

Commit 6d2de07

Browse files
deanrasheeddeanrasheed
committed
Fix EvalPlanQual rechecking during MERGE.
Under some circumstances, concurrent MERGE operations could lead to inconsistent results, that varied according the plan chosen. This was caused by a lack of rowmarks on the source relation, which meant that EvalPlanQual rechecking was not guaranteed to return the same source tuples when re-running the join query. Fix by ensuring that preprocess_rowmarks() sets up PlanRowMarks for all non-target relations used in MERGE, in the same way that it does for UPDATE and DELETE. Per bug #18103. Back-patch to v15, where MERGE was introduced. Dean Rasheed, reviewed by Richard Guo. Discussion: https://postgr.es/m/18103-c4386baab8e355e3%40postgresql.org
1 parent 5948664 commit 6d2de07

File tree

10 files changed

+237
-38
lines changed

10 files changed

+237
-38
lines changed

‎src/backend/executor/README

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ unnecessarily (for example, Sort does not rescan its input if no parameters
2626
of the input have changed, since it can just reread its stored sorted data).
2727

2828
For a SELECT, it is only necessary to deliver the top-level result tuples
29-
to the client. For INSERT/UPDATE/DELETE, the actual table modification
29+
to the client. For INSERT/UPDATE/DELETE/MERGE, the actual table modification
3030
operations happen in a top-level ModifyTable plan node. If the query
3131
includes a RETURNING clause, the ModifyTable node delivers the computed
3232
RETURNING rows as output, otherwise it returns nothing. Handling INSERT
@@ -353,8 +353,8 @@ EvalPlanQual (READ COMMITTED Update Checking)
353353
For simple SELECTs, the executor need only pay attention to tuples that are
354354
valid according to the snapshot seen by the current transaction (ie, they
355355
were inserted by a previously committed transaction, and not deleted by any
356-
previously committed transaction). However, for UPDATE and DELETE it is not
357-
cool to modify or delete a tuple that's been modified by an open or
356+
previously committed transaction). However, for UPDATE, DELETE, and MERGE it
357+
is not cool to modify or delete a tuple that's been modified by an open or
358358
concurrently-committed transaction. If we are running in SERIALIZABLE
359359
isolation level then we just raise an error when this condition is seen to
360360
occur. In READ COMMITTED isolation level, we must work a lot harder.
@@ -378,14 +378,14 @@ we're doing UPDATE). If no tuple is returned, then the modified tuple(s)
378378
fail the quals, so we ignore the current result tuple and continue the
379379
original query.
380380

381-
In UPDATE/DELETE, only the target relation needs to be handled this way.
381+
In UPDATE/DELETE/MERGE, only the target relation needs to be handled this way.
382382
In SELECT FOR UPDATE, there may be multiple relations flagged FOR UPDATE,
383383
so we obtain lock on the current tuple version in each such relation before
384384
executing the recheck.
385385

386386
It is also possible that there are relations in the query that are not
387-
to be locked (they are neither the UPDATE/DELETE target nor specified to
388-
be locked in SELECT FOR UPDATE/SHARE). When re-running the test query
387+
to be locked (they are neither the UPDATE/DELETE/MERGE target nor specified
388+
to be locked in SELECT FOR UPDATE/SHARE). When re-running the test query
389389
we want to use the same rows from these relations that were joined to
390390
the locked rows. For ordinary relations this can be implemented relatively
391391
cheaply by including the row TID in the join outputs and re-fetching that

‎src/backend/executor/nodeModifyTable.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4284,9 +4284,9 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
42844284

42854285
/*
42864286
* If we have any secondary relations in an UPDATE or DELETE, they need to
4287-
* be treated like non-locked relations in SELECT FOR UPDATE, ie, the
4288-
* EvalPlanQual mechanism needs to be told about them. Locate the
4289-
* relevant ExecRowMarks.
4287+
* be treated like non-locked relations in SELECT FOR UPDATE, i.e., the
4288+
* EvalPlanQual mechanism needs to be told about them. This also goes for
4289+
* the source relations in a MERGE. Locate the relevant ExecRowMarks.
42904290
*/
42914291
arowmarks = NIL;
42924292
foreach(l, node->rowMarks)

‎src/backend/optimizer/plan/planner.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2242,11 +2242,12 @@ preprocess_rowmarks(PlannerInfo *root)
22422242
else
22432243
{
22442244
/*
2245-
* We only need rowmarks for UPDATE, DELETE, or FOR [KEY]
2245+
* We only need rowmarks for UPDATE, DELETE, MERGE, or FOR [KEY]
22462246
* UPDATE/SHARE.
22472247
*/
22482248
if (parse->commandType != CMD_UPDATE &&
2249-
parse->commandType != CMD_DELETE)
2249+
parse->commandType != CMD_DELETE &&
2250+
parse->commandType != CMD_MERGE)
22502251
return;
22512252
}
22522253

‎src/include/nodes/execnodes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -721,8 +721,8 @@ typedef struct EState
721721
* ExecRowMark -
722722
* runtime representation of FOR [KEY] UPDATE/SHARE clauses
723723
*
724-
* When doing UPDATE, DELETE, or SELECT FOR [KEY] UPDATE/SHARE, we will have an
725-
* ExecRowMark for each non-target relation in the query (except inheritance
724+
* When doing UPDATE/DELETE/MERGE/SELECT FOR [KEY] UPDATE/SHARE, we will have
725+
* an ExecRowMark for each non-target relation in the query (except inheritance
726726
* parent RTEs, which can be ignored at runtime). Virtual relations such as
727727
* subqueries-in-FROM will have an ExecRowMark with relation == NULL. See
728728
* PlanRowMark for details about most of the fields. In addition to fields

‎src/include/nodes/plannodes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,7 @@ typedef struct Limit
13091309
* doing a separate remote query to lock each selected row is usually pretty
13101310
* unappealing, so early locking remains a credible design choice for FDWs.
13111311
*
1312-
* When doing UPDATE, DELETE, or SELECT FOR UPDATE/SHARE, we have to uniquely
1312+
* When doing UPDATE/DELETE/MERGE/SELECT FOR UPDATE/SHARE, we have to uniquely
13131313
* identify all the source rows, not only those from the target relations, so
13141314
* that we can perform EvalPlanQual rechecking at need. For plain tables we
13151315
* can just fetch the TID, much as for a target relation; this case is
@@ -1338,7 +1338,7 @@ typedef enum RowMarkType
13381338
* PlanRowMark -
13391339
* plan-time representation of FOR [KEY] UPDATE/SHARE clauses
13401340
*
1341-
* When doing UPDATE, DELETE, or SELECT FOR UPDATE/SHARE, we create a separate
1341+
* When doing UPDATE/DELETE/MERGE/SELECT FOR UPDATE/SHARE, we create a separate
13421342
* PlanRowMark node for each non-target relation in the query. Relations that
13431343
* are not specified as FOR UPDATE/SHARE are marked ROW_MARK_REFERENCE (if
13441344
* regular tables or supported foreign tables) or ROW_MARK_COPY (if not).

‎src/test/isolation/expected/merge-join.out

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: b1 m1 s1 c1 b2 m2 s2 c2
4+
step b1: BEGIN ISOLATION LEVEL READ COMMITTED;
5+
step m1: MERGE INTO tgt USING src ON tgt.id = src.id
6+
WHEN MATCHED THEN UPDATE SET val = src.val
7+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
8+
step s1: SELECT * FROM tgt;
9+
id|val
10+
--+---
11+
1| 10
12+
2| 20
13+
3| 30
14+
(3 rows)
15+
16+
step c1: COMMIT;
17+
step b2: BEGIN ISOLATION LEVEL READ COMMITTED;
18+
step m2: MERGE INTO tgt USING src ON tgt.id = src.id
19+
WHEN MATCHED THEN UPDATE SET val = src.val
20+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
21+
step s2: SELECT * FROM tgt;
22+
id|val
23+
--+---
24+
1| 10
25+
2| 20
26+
3| 30
27+
(3 rows)
28+
29+
step c2: COMMIT;
30+
31+
starting permutation: b1 b2 m1 hj ex m2 c1 c2 s1
32+
step b1: BEGIN ISOLATION LEVEL READ COMMITTED;
33+
step b2: BEGIN ISOLATION LEVEL READ COMMITTED;
34+
step m1: MERGE INTO tgt USING src ON tgt.id = src.id
35+
WHEN MATCHED THEN UPDATE SET val = src.val
36+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
37+
step hj: SET LOCAL enable_mergejoin = off; SET LOCAL enable_nestloop = off;
38+
step ex: EXPLAIN (verbose, costs off)
39+
MERGE INTO tgt USING src ON tgt.id = src.id
40+
WHEN MATCHED THEN UPDATE SET val = src.val
41+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
42+
QUERY PLAN
43+
---------------------------------------------------
44+
Merge on public.tgt
45+
-> Hash Left Join
46+
Output: tgt.ctid, src.val, src.id, src.ctid
47+
Inner Unique: true
48+
Hash Cond: (src.id = tgt.id)
49+
-> Seq Scan on public.src
50+
Output: src.val, src.id, src.ctid
51+
-> Hash
52+
Output: tgt.ctid, tgt.id
53+
-> Seq Scan on public.tgt
54+
Output: tgt.ctid, tgt.id
55+
(11 rows)
56+
57+
step m2: MERGE INTO tgt USING src ON tgt.id = src.id
58+
WHEN MATCHED THEN UPDATE SET val = src.val
59+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val); <waiting ...>
60+
step c1: COMMIT;
61+
step m2: <... completed>
62+
step c2: COMMIT;
63+
step s1: SELECT * FROM tgt;
64+
id|val
65+
--+---
66+
1| 10
67+
2| 20
68+
3| 30
69+
(3 rows)
70+
71+
72+
starting permutation: b1 b2 m1 mj ex m2 c1 c2 s1
73+
step b1: BEGIN ISOLATION LEVEL READ COMMITTED;
74+
step b2: BEGIN ISOLATION LEVEL READ COMMITTED;
75+
step m1: MERGE INTO tgt USING src ON tgt.id = src.id
76+
WHEN MATCHED THEN UPDATE SET val = src.val
77+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
78+
step mj: SET LOCAL enable_hashjoin = off; SET LOCAL enable_nestloop = off;
79+
step ex: EXPLAIN (verbose, costs off)
80+
MERGE INTO tgt USING src ON tgt.id = src.id
81+
WHEN MATCHED THEN UPDATE SET val = src.val
82+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
83+
QUERY PLAN
84+
---------------------------------------------------
85+
Merge on public.tgt
86+
-> Merge Left Join
87+
Output: tgt.ctid, src.val, src.id, src.ctid
88+
Inner Unique: true
89+
Merge Cond: (src.id = tgt.id)
90+
-> Index Scan using src_pkey on public.src
91+
Output: src.val, src.id, src.ctid
92+
-> Index Scan using tgt_pkey on public.tgt
93+
Output: tgt.ctid, tgt.id
94+
(9 rows)
95+
96+
step m2: MERGE INTO tgt USING src ON tgt.id = src.id
97+
WHEN MATCHED THEN UPDATE SET val = src.val
98+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val); <waiting ...>
99+
step c1: COMMIT;
100+
step m2: <... completed>
101+
step c2: COMMIT;
102+
step s1: SELECT * FROM tgt;
103+
id|val
104+
--+---
105+
1| 10
106+
2| 20
107+
3| 30
108+
(3 rows)
109+
110+
111+
starting permutation: b1 b2 m1 nl ex m2 c1 c2 s1
112+
step b1: BEGIN ISOLATION LEVEL READ COMMITTED;
113+
step b2: BEGIN ISOLATION LEVEL READ COMMITTED;
114+
step m1: MERGE INTO tgt USING src ON tgt.id = src.id
115+
WHEN MATCHED THEN UPDATE SET val = src.val
116+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
117+
step nl: SET LOCAL enable_hashjoin = off; SET LOCAL enable_mergejoin = off;
118+
step ex: EXPLAIN (verbose, costs off)
119+
MERGE INTO tgt USING src ON tgt.id = src.id
120+
WHEN MATCHED THEN UPDATE SET val = src.val
121+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val);
122+
QUERY PLAN
123+
---------------------------------------------------
124+
Merge on public.tgt
125+
-> Nested Loop Left Join
126+
Output: tgt.ctid, src.val, src.id, src.ctid
127+
Inner Unique: true
128+
-> Seq Scan on public.src
129+
Output: src.val, src.id, src.ctid
130+
-> Index Scan using tgt_pkey on public.tgt
131+
Output: tgt.ctid, tgt.id
132+
Index Cond: (tgt.id = src.id)
133+
(9 rows)
134+
135+
step m2: MERGE INTO tgt USING src ON tgt.id = src.id
136+
WHEN MATCHED THEN UPDATE SET val = src.val
137+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val); <waiting ...>
138+
step c1: COMMIT;
139+
step m2: <... completed>
140+
step c2: COMMIT;
141+
step s1: SELECT * FROM tgt;
142+
id|val
143+
--+---
144+
1| 10
145+
2| 20
146+
3| 30
147+
(3 rows)
148+

‎src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ test: merge-insert-update
5151
test: merge-delete
5252
test: merge-update
5353
test: merge-match-recheck
54+
test: merge-join
5455
test: delete-abort-savept
5556
test: delete-abort-savept-2
5657
test: aborted-keyrevoke

‎src/test/isolation/specs/merge-join.spec

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# MERGE JOIN
2+
#
3+
# This test checks the EPQ recheck mechanism during MERGE when joining to a
4+
# source table using different join methods, per bug #18103
5+
6+
setup
7+
{
8+
CREATE TABLE src (id int PRIMARY KEY, val int);
9+
CREATE TABLE tgt (id int PRIMARY KEY, val int);
10+
INSERT INTO src SELECT x, x*10 FROM generate_series(1,3) g(x);
11+
INSERT INTO tgt SELECT x, x FROM generate_series(1,3) g(x);
12+
}
13+
14+
teardown
15+
{
16+
DROP TABLE src, tgt;
17+
}
18+
19+
session s1
20+
step b1 { BEGIN ISOLATION LEVEL READ COMMITTED; }
21+
step m1 { MERGE INTO tgt USING src ON tgt.id = src.id
22+
WHEN MATCHED THEN UPDATE SET val = src.val
23+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val); }
24+
step s1 { SELECT * FROM tgt; }
25+
step c1 { COMMIT; }
26+
27+
session s2
28+
step b2 { BEGIN ISOLATION LEVEL READ COMMITTED; }
29+
step hj { SET LOCAL enable_mergejoin = off; SET LOCAL enable_nestloop = off; }
30+
step mj { SET LOCAL enable_hashjoin = off; SET LOCAL enable_nestloop = off; }
31+
step nl { SET LOCAL enable_hashjoin = off; SET LOCAL enable_mergejoin = off; }
32+
step ex { EXPLAIN (verbose, costs off)
33+
MERGE INTO tgt USING src ON tgt.id = src.id
34+
WHEN MATCHED THEN UPDATE SET val = src.val
35+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val); }
36+
step m2 { MERGE INTO tgt USING src ON tgt.id = src.id
37+
WHEN MATCHED THEN UPDATE SET val = src.val
38+
WHEN NOT MATCHED THEN INSERT VALUES (src.id, src.val); }
39+
step s2 { SELECT * FROM tgt; }
40+
step c2 { COMMIT; }
41+
42+
permutation b1 m1 s1 c1 b2 m2 s2 c2
43+
permutation b1 b2 m1 hj ex m2 c1 c2 s1
44+
permutation b1 b2 m1 mj ex m2 c1 c2 s1
45+
permutation b1 b2 m1 nl ex m2 c1 c2 s1

‎src/test/regress/expected/merge.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1829,11 +1829,11 @@ MERGE INTO pa_target t USING pa_source s ON t.tid = s.sid
18291829
Merge on public.pa_target t
18301830
Merge on public.pa_targetp t_1
18311831
-> Hash Left Join
1832-
Output: s.sid, t_1.tableoid, t_1.ctid
1832+
Output: s.sid, s.ctid, t_1.tableoid, t_1.ctid
18331833
Inner Unique: true
18341834
Hash Cond: (s.sid = t_1.tid)
18351835
-> Seq Scan on public.pa_source s
1836-
Output: s.sid
1836+
Output: s.sid, s.ctid
18371837
-> Hash
18381838
Output: t_1.tid, t_1.tableoid, t_1.ctid
18391839
-> Seq Scan on public.pa_targetp t_1
@@ -1859,11 +1859,11 @@ MERGE INTO pa_target t USING pa_source s ON t.tid = s.sid
18591859
--------------------------------------------
18601860
Merge on public.pa_target t
18611861
-> Hash Left Join
1862-
Output: s.sid, t.ctid
1862+
Output: s.sid, s.ctid, t.ctid
18631863
Inner Unique: true
18641864
Hash Cond: (s.sid = t.tid)
18651865
-> Seq Scan on public.pa_source s
1866-
Output: s.sid
1866+
Output: s.sid, s.ctid
18671867
-> Hash
18681868
Output: t.tid, t.ctid
18691869
-> Result

0 commit comments

Comments
 (0)