diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c index 9b7f470ee28..f61b3abf326 100644 --- a/src/backend/executor/nodeIndexonlyscan.c +++ b/src/backend/executor/nodeIndexonlyscan.c @@ -421,11 +421,39 @@ ExecEndIndexOnlyScan(IndexOnlyScanState *node) /* ---------------------------------------------------------------- * ExecIndexOnlyMarkPos + * + * Note: we assume that no caller attempts to set a mark before having read + * at least one tuple. Otherwise, ioss_ScanDesc might still be NULL. * ---------------------------------------------------------------- */ void ExecIndexOnlyMarkPos(IndexOnlyScanState *node) { + EState *estate = node->ss.ps.state; + + if (estate->es_epqTuple != NULL) + { + /* + * We are inside an EvalPlanQual recheck. If a test tuple exists for + * this relation, then we shouldn't access the index at all. We would + * instead need to save, and later restore, the state of the + * es_epqScanDone flag, so that re-fetching the test tuple is + * possible. However, given the assumption that no caller sets a mark + * at the start of the scan, we can only get here with es_epqScanDone + * already set, and so no state need be saved. + */ + Index scanrelid = ((Scan *) node->ss.ps.plan)->scanrelid; + + Assert(scanrelid > 0); + if (estate->es_epqTupleSet[scanrelid - 1]) + { + /* Verify the claim above */ + if (!estate->es_epqScanDone[scanrelid - 1]) + elog(ERROR, "unexpected ExecIndexOnlyMarkPos call in EPQ recheck"); + return; + } + } + index_markpos(node->ioss_ScanDesc); } @@ -436,6 +464,23 @@ ExecIndexOnlyMarkPos(IndexOnlyScanState *node) void ExecIndexOnlyRestrPos(IndexOnlyScanState *node) { + EState *estate = node->ss.ps.state; + + if (estate->es_epqTuple != NULL) + { + /* See comments in ExecIndexOnlyMarkPos */ + Index scanrelid = ((Scan *) node->ss.ps.plan)->scanrelid; + + Assert(scanrelid > 0); + if (estate->es_epqTupleSet[scanrelid - 1]) + { + /* Verify the claim above */ + if (!estate->es_epqScanDone[scanrelid - 1]) + elog(ERROR, "unexpected ExecIndexOnlyRestrPos call in EPQ recheck"); + return; + } + } + index_restrpos(node->ioss_ScanDesc); } diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index 54fafa5033f..eed69a0c665 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -849,11 +849,39 @@ ExecEndIndexScan(IndexScanState *node) /* ---------------------------------------------------------------- * ExecIndexMarkPos + * + * Note: we assume that no caller attempts to set a mark before having read + * at least one tuple. Otherwise, iss_ScanDesc might still be NULL. * ---------------------------------------------------------------- */ void ExecIndexMarkPos(IndexScanState *node) { + EState *estate = node->ss.ps.state; + + if (estate->es_epqTuple != NULL) + { + /* + * We are inside an EvalPlanQual recheck. If a test tuple exists for + * this relation, then we shouldn't access the index at all. We would + * instead need to save, and later restore, the state of the + * es_epqScanDone flag, so that re-fetching the test tuple is + * possible. However, given the assumption that no caller sets a mark + * at the start of the scan, we can only get here with es_epqScanDone + * already set, and so no state need be saved. + */ + Index scanrelid = ((Scan *) node->ss.ps.plan)->scanrelid; + + Assert(scanrelid > 0); + if (estate->es_epqTupleSet[scanrelid - 1]) + { + /* Verify the claim above */ + if (!estate->es_epqScanDone[scanrelid - 1]) + elog(ERROR, "unexpected ExecIndexMarkPos call in EPQ recheck"); + return; + } + } + index_markpos(node->iss_ScanDesc); } @@ -864,6 +892,23 @@ ExecIndexMarkPos(IndexScanState *node) void ExecIndexRestrPos(IndexScanState *node) { + EState *estate = node->ss.ps.state; + + if (estate->es_epqTuple != NULL) + { + /* See comments in ExecIndexMarkPos */ + Index scanrelid = ((Scan *) node->ss.ps.plan)->scanrelid; + + Assert(scanrelid > 0); + if (estate->es_epqTupleSet[scanrelid - 1]) + { + /* Verify the claim above */ + if (!estate->es_epqScanDone[scanrelid - 1]) + elog(ERROR, "unexpected ExecIndexRestrPos call in EPQ recheck"); + return; + } + } + index_restrpos(node->iss_ScanDesc); } diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c index b52946f180a..ec5f82f6a92 100644 --- a/src/backend/executor/nodeMergejoin.c +++ b/src/backend/executor/nodeMergejoin.c @@ -1502,6 +1502,10 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) * * Currently, only Material wants the extra MARKs, and it will be helpful * only if eflags doesn't specify REWIND. + * + * Note that for IndexScan and IndexOnlyScan, it is *necessary* that we + * not set mj_ExtraMarks; otherwise we might attempt to set a mark before + * the first inner tuple, which they do not support. */ if (IsA(innerPlan(node), Material) && (eflags & EXEC_FLAG_REWIND) == 0 && diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out index 10c784a05f1..eb40717679c 100644 --- a/src/test/isolation/expected/eval-plan-qual.out +++ b/src/test/isolation/expected/eval-plan-qual.out @@ -184,3 +184,36 @@ step readwcte: <... completed> id value 1 tableAValue2 + +starting permutation: wrjt selectjoinforupdate c2 c1 +step wrjt: UPDATE jointest SET data = 42 WHERE id = 7; +step selectjoinforupdate: + set enable_nestloop to 0; + set enable_hashjoin to 0; + set enable_seqscan to 0; + explain (costs off) + select * from jointest a join jointest b on a.id=b.id for update; + select * from jointest a join jointest b on a.id=b.id for update; + +step c2: COMMIT; +step selectjoinforupdate: <... completed> +QUERY PLAN + +LockRows + -> Merge Join + Merge Cond: (a.id = b.id) + -> Index Scan using jointest_id_idx on jointest a + -> Index Scan using jointest_id_idx on jointest b +id data id data + +1 0 1 0 +2 0 2 0 +3 0 3 0 +4 0 4 0 +5 0 5 0 +6 0 6 0 +7 42 7 42 +8 0 8 0 +9 0 9 0 +10 0 10 0 +step c1: COMMIT; diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec index 7ff6f6b8cc9..d2b34ec7cc2 100644 --- a/src/test/isolation/specs/eval-plan-qual.spec +++ b/src/test/isolation/specs/eval-plan-qual.spec @@ -21,13 +21,16 @@ setup CREATE TABLE table_b (id integer, value text); INSERT INTO table_a VALUES (1, 'tableAValue'); INSERT INTO table_b VALUES (1, 'tableBValue'); + + CREATE TABLE jointest AS SELECT generate_series(1,10) AS id, 0 AS data; + CREATE INDEX ON jointest(id); } teardown { DROP TABLE accounts; DROP TABLE p CASCADE; - DROP TABLE table_a, table_b; + DROP TABLE table_a, table_b, jointest; } session "s1" @@ -78,6 +81,17 @@ step "updateforss" { UPDATE table_b SET value = 'newTableBValue' WHERE id = 1; } +# these tests exercise mark/restore during EPQ recheck, cf bug #15032 + +step "selectjoinforupdate" { + set enable_nestloop to 0; + set enable_hashjoin to 0; + set enable_seqscan to 0; + explain (costs off) + select * from jointest a join jointest b on a.id=b.id for update; + select * from jointest a join jointest b on a.id=b.id for update; +} + session "s2" setup { BEGIN ISOLATION LEVEL READ COMMITTED; } @@ -104,6 +118,7 @@ step "readforss" { WHERE ta.id = 1 FOR UPDATE OF ta; } step "wrtwcte" { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; } +step "wrjt" { UPDATE jointest SET data = 42 WHERE id = 7; } step "c2" { COMMIT; } session "s3" @@ -135,3 +150,4 @@ permutation "wx2" "partiallock" "c2" "c1" "read" permutation "wx2" "lockwithvalues" "c2" "c1" "read" permutation "updateforss" "readforss" "c1" "c2" permutation "wrtwcte" "readwcte" "c1" "c2" +permutation "wrjt" "selectjoinforupdate" "c2" "c1"