Fix corner case where SELECT FOR UPDATE could return a row twice.

In READ COMMITTED mode, if a SELECT FOR UPDATE discovers it has to redo
WHERE-clause checking on rows that have been updated since the SELECT's
snapshot, it invokes EvalPlanQual processing to do that.  If this first
occurs within a non-first child table of an inheritance tree, the previous
coding could accidentally re-return a matching row from an earlier,
already-scanned child table.  (And, to add insult to injury, I think this
could make it miss returning a row that should have been returned, if the
updated row that this happens on should still have passed the WHERE qual.)
Per report from Kyotaro Horiguchi; the added isolation test is based on his
test case.

This has been broken for quite awhile, so back-patch to all supported
branches.
This commit is contained in:
Tom Lane 2014-12-11 19:37:00 -05:00
parent 2646d2d4a9
commit 2db576ba8c
3 changed files with 61 additions and 0 deletions

View File

@ -194,7 +194,29 @@ lnext:
*/ */
if (!epq_started) if (!epq_started)
{ {
ListCell *lc2;
EvalPlanQualBegin(&node->lr_epqstate, estate); EvalPlanQualBegin(&node->lr_epqstate, estate);
/*
* Ensure that rels with already-visited rowmarks are told
* not to return tuples during the first EPQ test. We can
* exit this loop once it reaches the current rowmark;
* rels appearing later in the list will be set up
* correctly by the EvalPlanQualSetTuple call at the top
* of the loop.
*/
foreach(lc2, node->lr_arowMarks)
{
ExecAuxRowMark *aerm2 = (ExecAuxRowMark *) lfirst(lc2);
if (lc2 == lc)
break;
EvalPlanQualSetTuple(&node->lr_epqstate,
aerm2->rowmark->rti,
NULL);
}
epq_started = true; epq_started = true;
} }

View File

@ -49,3 +49,26 @@ accountid balance
checking 600 checking 600
savings 2334 savings 2334
starting permutation: readp1 writep1 readp2 c1 c2
step readp1: SELECT tableoid::regclass, ctid, * FROM p WHERE b IN (0, 1) AND c = 0 FOR UPDATE;
tableoid ctid a b c
c1 (0,1) 0 0 0
c1 (0,4) 0 1 0
c2 (0,1) 1 0 0
c2 (0,4) 1 1 0
c3 (0,1) 2 0 0
c3 (0,4) 2 1 0
step writep1: UPDATE p SET b = -1 WHERE a = 1 AND b = 1 AND c = 0;
step readp2: SELECT tableoid::regclass, ctid, * FROM p WHERE b IN (0, 1) AND c = 0 FOR UPDATE; <waiting ...>
step c1: COMMIT;
step readp2: <... completed>
tableoid ctid a b c
c1 (0,1) 0 0 0
c1 (0,4) 0 1 0
c2 (0,1) 1 0 0
c3 (0,1) 2 0 0
c3 (0,4) 2 1 0
step c2: COMMIT;

View File

@ -8,11 +8,20 @@ setup
{ {
CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null); CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null);
INSERT INTO accounts VALUES ('checking', 600), ('savings', 600); INSERT INTO accounts VALUES ('checking', 600), ('savings', 600);
CREATE TABLE p (a int, b int, c int);
CREATE TABLE c1 () INHERITS (p);
CREATE TABLE c2 () INHERITS (p);
CREATE TABLE c3 () INHERITS (p);
INSERT INTO c1 SELECT 0, a / 3, a % 3 FROM generate_series(0, 9) a;
INSERT INTO c2 SELECT 1, a / 3, a % 3 FROM generate_series(0, 9) a;
INSERT INTO c3 SELECT 2, a / 3, a % 3 FROM generate_series(0, 9) a;
} }
teardown teardown
{ {
DROP TABLE accounts; DROP TABLE accounts;
DROP TABLE p CASCADE;
} }
session "s1" session "s1"
@ -30,6 +39,11 @@ step "upsert1" {
INSERT INTO accounts SELECT 'savings', 500 INSERT INTO accounts SELECT 'savings', 500
WHERE NOT EXISTS (SELECT 1 FROM upsert); WHERE NOT EXISTS (SELECT 1 FROM upsert);
} }
# tests with table p check inheritance cases, specifically a bug where
# nodeLockRows did the wrong thing when the first updated tuple was in
# a non-first child table
step "readp1" { SELECT tableoid::regclass, ctid, * FROM p WHERE b IN (0, 1) AND c = 0 FOR UPDATE; }
step "writep1" { UPDATE p SET b = -1 WHERE a = 1 AND b = 1 AND c = 0; }
step "c1" { COMMIT; } step "c1" { COMMIT; }
session "s2" session "s2"
@ -44,6 +58,7 @@ step "upsert2" {
INSERT INTO accounts SELECT 'savings', 1234 INSERT INTO accounts SELECT 'savings', 1234
WHERE NOT EXISTS (SELECT 1 FROM upsert); WHERE NOT EXISTS (SELECT 1 FROM upsert);
} }
step "readp2" { SELECT tableoid::regclass, ctid, * FROM p WHERE b IN (0, 1) AND c = 0 FOR UPDATE; }
step "c2" { COMMIT; } step "c2" { COMMIT; }
session "s3" session "s3"
@ -54,3 +69,4 @@ teardown { COMMIT; }
permutation "wx1" "wx2" "c1" "c2" "read" permutation "wx1" "wx2" "c1" "c2" "read"
permutation "wy1" "wy2" "c1" "c2" "read" permutation "wy1" "wy2" "c1" "c2" "read"
permutation "upsert1" "upsert2" "c1" "c2" "read" permutation "upsert1" "upsert2" "c1" "c2" "read"
permutation "readp1" "writep1" "readp2" "c1" "c2"