diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b3ce4bae530..42fe8f8fb65 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2343,7 +2343,7 @@ ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist) * relation - table containing tuple * rti - rangetable index of table containing tuple * inputslot - tuple for processing - this can be the slot from - * EvalPlanQualSlot(), for the increased efficiency. + * EvalPlanQualSlot() for this rel, for increased efficiency. * * This tests whether the tuple in inputslot still matches the relevant * quals. For that result to be useful, typically the input tuple has to be @@ -2377,6 +2377,14 @@ EvalPlanQual(EPQState *epqstate, Relation relation, if (testslot != inputslot) ExecCopySlot(testslot, inputslot); + /* + * Mark that an EPQ tuple is available for this relation. (If there is + * more than one result relation, the others remain marked as having no + * tuple available.) + */ + epqstate->relsubs_done[rti - 1] = false; + epqstate->epqExtra->relsubs_blocked[rti - 1] = false; + /* * Run the EPQ query. We assume it will return at most one tuple. */ @@ -2393,11 +2401,12 @@ EvalPlanQual(EPQState *epqstate, Relation relation, ExecMaterializeSlot(slot); /* - * Clear out the test tuple. This is needed in case the EPQ query is - * re-used to test a tuple for a different relation. (Not clear that can - * really happen, but let's be safe.) + * Clear out the test tuple, and mark that no tuple is available here. + * This is needed in case the EPQ state is re-used to test a tuple for a + * different target relation. */ ExecClearTuple(testslot); + epqstate->epqExtra->relsubs_blocked[rti - 1] = true; return slot; } @@ -2412,12 +2421,34 @@ EvalPlanQual(EPQState *epqstate, Relation relation, void EvalPlanQualInit(EPQState *epqstate, EState *parentestate, Plan *subplan, List *auxrowmarks, int epqParam) +{ + EvalPlanQualInitExt(epqstate, parentestate, + subplan, auxrowmarks, epqParam, NIL); +} + +/* + * EvalPlanQualInitExt -- same, but allow specification of resultRelations + * + * If the caller intends to use EvalPlanQual(), resultRelations should be + * a list of RT indexes of potential target relations for EvalPlanQual(), + * and we will arrange that the other listed relations don't return any + * tuple during an EvalPlanQual() call. Otherwise resultRelations + * should be NIL. + */ +void +EvalPlanQualInitExt(EPQState *epqstate, EState *parentestate, + Plan *subplan, List *auxrowmarks, + int epqParam, List *resultRelations) { Index rtsize = parentestate->es_range_table_size; + /* create some extra space to avoid ABI break */ + epqstate->epqExtra = (EPQStateExtra *) palloc(sizeof(EPQStateExtra)); + /* initialize data not changing over EPQState's lifetime */ epqstate->parentestate = parentestate; epqstate->epqParam = epqParam; + epqstate->epqExtra->resultRelations = resultRelations; /* * Allocate space to reference a slot for each potential rti - do so now @@ -2426,7 +2457,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *parentestate, * that *may* need EPQ later, without forcing the overhead of * EvalPlanQualBegin(). */ - epqstate->tuple_table = NIL; + epqstate->epqExtra->tuple_table = NIL; epqstate->relsubs_slot = (TupleTableSlot **) palloc0(rtsize * sizeof(TupleTableSlot *)); @@ -2440,6 +2471,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *parentestate, epqstate->recheckplanstate = NULL; epqstate->relsubs_rowmark = NULL; epqstate->relsubs_done = NULL; + epqstate->epqExtra->relsubs_blocked = NULL; } /* @@ -2480,7 +2512,7 @@ EvalPlanQualSlot(EPQState *epqstate, MemoryContext oldcontext; oldcontext = MemoryContextSwitchTo(epqstate->parentestate->es_query_cxt); - *slot = table_slot_create(relation, &epqstate->tuple_table); + *slot = table_slot_create(relation, &epqstate->epqExtra->tuple_table); MemoryContextSwitchTo(oldcontext); } @@ -2637,7 +2669,13 @@ EvalPlanQualBegin(EPQState *epqstate) Index rtsize = parentestate->es_range_table_size; PlanState *rcplanstate = epqstate->recheckplanstate; - MemSet(epqstate->relsubs_done, 0, rtsize * sizeof(bool)); + /* + * Reset the relsubs_done[] flags to equal relsubs_blocked[], so that + * the EPQ run will never attempt to fetch tuples from blocked target + * relations. + */ + memcpy(epqstate->relsubs_done, epqstate->epqExtra->relsubs_blocked, + rtsize * sizeof(bool)); /* Recopy current values of parent parameters */ if (parentestate->es_plannedstmt->paramExecTypes != NIL) @@ -2804,10 +2842,22 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) } /* - * Initialize per-relation EPQ tuple states to not-fetched. + * Initialize per-relation EPQ tuple states. Result relations, if any, + * get marked as blocked; others as not-fetched. */ - epqstate->relsubs_done = (bool *) - palloc0(rtsize * sizeof(bool)); + epqstate->relsubs_done = palloc_array(bool, rtsize); + epqstate->epqExtra->relsubs_blocked = palloc0_array(bool, rtsize); + + foreach(l, epqstate->epqExtra->resultRelations) + { + int rtindex = lfirst_int(l); + + Assert(rtindex > 0 && rtindex <= rtsize); + epqstate->epqExtra->relsubs_blocked[rtindex - 1] = true; + } + + memcpy(epqstate->relsubs_done, epqstate->epqExtra->relsubs_blocked, + rtsize * sizeof(bool)); /* * Initialize the private state information for all the nodes in the part @@ -2844,12 +2894,12 @@ EvalPlanQualEnd(EPQState *epqstate) * We may have a tuple table, even if EPQ wasn't started, because we allow * use of EvalPlanQualSlot() without calling EvalPlanQualBegin(). */ - if (epqstate->tuple_table != NIL) + if (epqstate->epqExtra->tuple_table != NIL) { memset(epqstate->relsubs_slot, 0, rtsize * sizeof(TupleTableSlot *)); - ExecResetTupleTable(epqstate->tuple_table, true); - epqstate->tuple_table = NIL; + ExecResetTupleTable(epqstate->epqExtra->tuple_table, true); + epqstate->epqExtra->tuple_table = NIL; } /* EPQ wasn't started, nothing further to do */ @@ -2883,4 +2933,5 @@ EvalPlanQualEnd(EPQState *epqstate) epqstate->recheckplanstate = NULL; epqstate->relsubs_rowmark = NULL; epqstate->relsubs_done = NULL; + epqstate->epqExtra->relsubs_blocked = NULL; } diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c index 69ab34573ec..dc835dee0ec 100644 --- a/src/backend/executor/execScan.c +++ b/src/backend/executor/execScan.c @@ -69,13 +69,12 @@ ExecScanFetch(ScanState *node, else if (epqstate->relsubs_done[scanrelid - 1]) { /* - * Return empty slot, as we already performed an EPQ substitution - * for this relation. + * Return empty slot, as either there is no EPQ tuple for this rel + * or we already returned it. */ TupleTableSlot *slot = node->ss_ScanTupleSlot; - /* Return empty slot, as we already returned a tuple */ return ExecClearTuple(slot); } else if (epqstate->relsubs_slot[scanrelid - 1] != NULL) @@ -88,7 +87,7 @@ ExecScanFetch(ScanState *node, Assert(epqstate->relsubs_rowmark[scanrelid - 1] == NULL); - /* Mark to remember that we shouldn't return more */ + /* Mark to remember that we shouldn't return it again */ epqstate->relsubs_done[scanrelid - 1] = true; /* Return empty slot if we haven't got a test tuple */ @@ -306,14 +305,18 @@ ExecScanReScan(ScanState *node) */ ExecClearTuple(node->ss_ScanTupleSlot); - /* Rescan EvalPlanQual tuple if we're inside an EvalPlanQual recheck */ + /* + * Rescan EvalPlanQual tuple(s) if we're inside an EvalPlanQual recheck. + * But don't lose the "blocked" status of blocked target relations. + */ if (estate->es_epq_active != NULL) { EPQState *epqstate = estate->es_epq_active; Index scanrelid = ((Scan *) node->ps.plan)->scanrelid; if (scanrelid > 0) - epqstate->relsubs_done[scanrelid - 1] = false; + epqstate->relsubs_done[scanrelid - 1] = + epqstate->epqExtra->relsubs_blocked[scanrelid - 1]; else { Bitmapset *relids; @@ -335,7 +338,8 @@ ExecScanReScan(ScanState *node) while ((rtindex = bms_next_member(relids, rtindex)) >= 0) { Assert(rtindex > 0); - epqstate->relsubs_done[rtindex - 1] = false; + epqstate->relsubs_done[rtindex - 1] = + epqstate->epqExtra->relsubs_blocked[rtindex - 1]; } } } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 0227252b88e..c1f289a0fcb 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2880,7 +2880,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) } /* set up epqstate with dummy subplan data for the moment */ - EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL, node->epqParam); + EvalPlanQualInitExt(&mtstate->mt_epqstate, estate, NULL, NIL, + node->epqParam, node->resultRelations); mtstate->fireBSTriggers = true; /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 3dc03c913e3..cc97cf8cc7f 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -219,6 +219,9 @@ extern TupleTableSlot *EvalPlanQual(EPQState *epqstate, Relation relation, Index rti, TupleTableSlot *testslot); extern void EvalPlanQualInit(EPQState *epqstate, EState *parentestate, Plan *subplan, List *auxrowmarks, int epqParam); +extern void EvalPlanQualInitExt(EPQState *epqstate, EState *parentestate, + Plan *subplan, List *auxrowmarks, + int epqParam, List *resultRelations); extern void EvalPlanQualSetPlan(EPQState *epqstate, Plan *subplan, List *auxrowmarks); extern TupleTableSlot *EvalPlanQualSlot(EPQState *epqstate, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index ee5ad3c0583..c8286cc4f38 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1121,17 +1121,17 @@ typedef struct PlanState */ typedef struct EPQState { - /* Initialized at EvalPlanQualInit() time: */ - + /* These are initialized by EvalPlanQualInit() and do not change later: */ EState *parentestate; /* main query's EState */ int epqParam; /* ID of Param to force scan node re-eval */ + struct EPQStateExtra *epqExtra; /* extension pointer to avoid ABI break */ /* - * Tuples to be substituted by scan nodes. They need to set up, before - * calling EvalPlanQual()/EvalPlanQualNext(), into the slot returned by - * EvalPlanQualSlot(scanrelid). The array is indexed by scanrelid - 1. + * relsubs_slot[scanrelid - 1] holds the EPQ test tuple to be returned by + * the scan node for the scanrelid'th RT index, in place of performing an + * actual table scan. Callers should use EvalPlanQualSlot() to fetch + * these slots. */ - List *tuple_table; /* tuple table for relsubs_slot */ TupleTableSlot **relsubs_slot; /* @@ -1163,8 +1163,9 @@ typedef struct EPQState ExecAuxRowMark **relsubs_rowmark; /* - * True if a relation's EPQ tuple has been fetched for relation, indexed - * by scanrelid - 1. + * relsubs_done[scanrelid - 1] is true if there is no EPQ tuple for this + * target relation or it has already been fetched in the current scan of + * this target relation within the current EvalPlanQual test. */ bool *relsubs_done; @@ -1172,6 +1173,25 @@ typedef struct EPQState } EPQState; +/* + * To avoid an ABI-breaking change in the size of EPQState in back branches, + * we create one of these during EvalPlanQualInit. + */ +typedef struct EPQStateExtra +{ + List *resultRelations; /* integer list of RT indexes, or NIL */ + List *tuple_table; /* tuple table for relsubs_slot */ + + /* + * relsubs_blocked[scanrelid - 1] is true if there is no EPQ tuple for + * this target relation during the current EvalPlanQual test. We keep + * these flags set for all relids listed in resultRelations, but + * transiently clear the one for the relation whose tuple is actually + * passed to EvalPlanQual(). + */ + bool *relsubs_blocked; +} EPQStateExtra; + /* ---------------- * ResultState information * ---------------- diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out index 6a2a9a4ac71..feca9ede3db 100644 --- a/src/test/isolation/expected/eval-plan-qual.out +++ b/src/test/isolation/expected/eval-plan-qual.out @@ -842,6 +842,90 @@ step c1: COMMIT; step writep3b: <... completed> step c2: COMMIT; +starting permutation: writep4a writep4b c1 c2 readp +step writep4a: UPDATE p SET c = 4 WHERE c = 0; +step writep4b: UPDATE p SET b = -4 WHERE c = 0; +step c1: COMMIT; +step writep4b: <... completed> +step c2: COMMIT; +step readp: SELECT tableoid::regclass, ctid, * FROM p; +tableoid|ctid |a|b|c +--------+------+-+-+- +c1 |(0,2) |0|0|1 +c1 |(0,3) |0|0|2 +c1 |(0,5) |0|1|1 +c1 |(0,6) |0|1|2 +c1 |(0,8) |0|2|1 +c1 |(0,9) |0|2|2 +c1 |(0,11)|0|0|4 +c1 |(0,12)|0|1|4 +c1 |(0,13)|0|2|4 +c1 |(0,14)|0|3|4 +c2 |(0,2) |1|0|1 +c2 |(0,3) |1|0|2 +c2 |(0,5) |1|1|1 +c2 |(0,6) |1|1|2 +c2 |(0,8) |1|2|1 +c2 |(0,9) |1|2|2 +c2 |(0,11)|1|0|4 +c2 |(0,12)|1|1|4 +c2 |(0,13)|1|2|4 +c2 |(0,14)|1|3|4 +c3 |(0,2) |2|0|1 +c3 |(0,3) |2|0|2 +c3 |(0,5) |2|1|1 +c3 |(0,6) |2|1|2 +c3 |(0,8) |2|2|1 +c3 |(0,9) |2|2|2 +c3 |(0,11)|2|0|4 +c3 |(0,12)|2|1|4 +c3 |(0,13)|2|2|4 +c3 |(0,14)|2|3|4 +(30 rows) + + +starting permutation: writep4a deletep4 c1 c2 readp +step writep4a: UPDATE p SET c = 4 WHERE c = 0; +step deletep4: DELETE FROM p WHERE c = 0; +step c1: COMMIT; +step deletep4: <... completed> +step c2: COMMIT; +step readp: SELECT tableoid::regclass, ctid, * FROM p; +tableoid|ctid |a|b|c +--------+------+-+-+- +c1 |(0,2) |0|0|1 +c1 |(0,3) |0|0|2 +c1 |(0,5) |0|1|1 +c1 |(0,6) |0|1|2 +c1 |(0,8) |0|2|1 +c1 |(0,9) |0|2|2 +c1 |(0,11)|0|0|4 +c1 |(0,12)|0|1|4 +c1 |(0,13)|0|2|4 +c1 |(0,14)|0|3|4 +c2 |(0,2) |1|0|1 +c2 |(0,3) |1|0|2 +c2 |(0,5) |1|1|1 +c2 |(0,6) |1|1|2 +c2 |(0,8) |1|2|1 +c2 |(0,9) |1|2|2 +c2 |(0,11)|1|0|4 +c2 |(0,12)|1|1|4 +c2 |(0,13)|1|2|4 +c2 |(0,14)|1|3|4 +c3 |(0,2) |2|0|1 +c3 |(0,3) |2|0|2 +c3 |(0,5) |2|1|1 +c3 |(0,6) |2|1|2 +c3 |(0,8) |2|2|1 +c3 |(0,9) |2|2|2 +c3 |(0,11)|2|0|4 +c3 |(0,12)|2|1|4 +c3 |(0,13)|2|2|4 +c3 |(0,14)|2|3|4 +(30 rows) + + starting permutation: wx2 partiallock c2 c1 read step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; balance @@ -1104,22 +1188,41 @@ subid|id (1 row) +starting permutation: simplepartupdate conditionalpartupdate c1 c2 read_part +step simplepartupdate: + update parttbl set b = b + 10; + +step conditionalpartupdate: + update parttbl set c = -c where b < 10; + +step c1: COMMIT; +step conditionalpartupdate: <... completed> +step c2: COMMIT; +step read_part: SELECT * FROM parttbl ORDER BY a, c; +a| b|c| d +-+--+-+-- +1|11|1|12 +2|12|2|14 +(2 rows) + + starting permutation: simplepartupdate complexpartupdate c1 c2 read_part step simplepartupdate: update parttbl set b = b + 10; step complexpartupdate: with u as (update parttbl set b = b + 1 returning parttbl.*) - update parttbl set b = u.b + 100 from u; + update parttbl p set b = u.b + 100 from u where p.a = u.a; step c1: COMMIT; step complexpartupdate: <... completed> step c2: COMMIT; -step read_part: SELECT * FROM parttbl ORDER BY a; +step read_part: SELECT * FROM parttbl ORDER BY a, c; a| b|c| d -+--+-+-- 1|12|1|13 -(1 row) +2|13|2|15 +(2 rows) starting permutation: simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2 read_part @@ -1139,11 +1242,12 @@ step c1: COMMIT; step complexpartupdate_route_err1: <... completed> ERROR: tuple to be locked was already moved to another partition due to concurrent update step c2: COMMIT; -step read_part: SELECT * FROM parttbl ORDER BY a; +step read_part: SELECT * FROM parttbl ORDER BY a, c; a|b|c|d -+-+-+- 2|1|1|3 -(1 row) +2|2|2|4 +(2 rows) starting permutation: simplepartupdate_noroute complexpartupdate_route c1 c2 read_part @@ -1167,11 +1271,12 @@ a|b|c|d (1 row) step c2: COMMIT; -step read_part: SELECT * FROM parttbl ORDER BY a; +step read_part: SELECT * FROM parttbl ORDER BY a, c; a|b|c|d -+-+-+- 2|2|1|4 -(1 row) +2|2|2|4 +(2 rows) starting permutation: simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2 read_part @@ -1195,9 +1300,10 @@ a|b|c|d (1 row) step c2: COMMIT; -step read_part: SELECT * FROM parttbl ORDER BY a; +step read_part: SELECT * FROM parttbl ORDER BY a, c; a|b|c|d -+-+-+- 1|2|1|3 -(1 row) +2|2|2|4 +(2 rows) diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec index 3a515223176..ac7d3532803 100644 --- a/src/test/isolation/specs/eval-plan-qual.spec +++ b/src/test/isolation/specs/eval-plan-qual.spec @@ -38,7 +38,7 @@ setup d int GENERATED ALWAYS AS (a + b) STORED) PARTITION BY LIST (a); CREATE TABLE parttbl1 PARTITION OF parttbl FOR VALUES IN (1); CREATE TABLE parttbl2 PARTITION OF parttbl FOR VALUES IN (2); - INSERT INTO parttbl VALUES (1, 1, 1); + INSERT INTO parttbl VALUES (1, 1, 1), (2, 2, 2); CREATE TABLE another_parttbl (a int, b int, c int) PARTITION BY LIST (a); CREATE TABLE another_parttbl1 PARTITION OF another_parttbl FOR VALUES IN (1); @@ -100,11 +100,15 @@ step upsert1 { # when the first updated tuple was in a non-first child table. # writep2/returningp1 tests a memory allocation issue # writep3a/writep3b tests updates touching more than one table +# writep4a/writep4b tests a case where matches in another table confused EPQ +# writep4a/deletep4 tests the same case in the DELETE path +step readp { SELECT tableoid::regclass, ctid, * FROM p; } 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 writep2 { UPDATE p SET b = -b WHERE a = 1 AND c = 0; } step writep3a { UPDATE p SET b = -b WHERE c = 0; } +step writep4a { UPDATE p SET c = 4 WHERE c = 0; } step c1 { COMMIT; } step r1 { ROLLBACK; } @@ -208,6 +212,8 @@ step returningp1 { SELECT * FROM u; } step writep3b { UPDATE p SET b = -b WHERE c = 0; } +step writep4b { UPDATE p SET b = -4 WHERE c = 0; } +step deletep4 { DELETE FROM p WHERE c = 0; } step readforss { SELECT ta.id AS ta_id, ta.value AS ta_value, (SELECT ROW(tb.id, tb.value) @@ -224,9 +230,14 @@ step updateforcip3 { } step wrtwcte { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; } step wrjt { UPDATE jointest SET data = 42 WHERE id = 7; } + +step conditionalpartupdate { + update parttbl set c = -c where b < 10; +} + step complexpartupdate { with u as (update parttbl set b = b + 1 returning parttbl.*) - update parttbl set b = u.b + 100 from u; + update parttbl p set b = u.b + 100 from u where p.a = u.a; } step complexpartupdate_route_err1 { @@ -275,7 +286,7 @@ setup { BEGIN ISOLATION LEVEL READ COMMITTED; } step read { SELECT * FROM accounts ORDER BY accountid; } step read_ext { SELECT * FROM accounts_ext ORDER BY accountid; } step read_a { SELECT * FROM table_a ORDER BY id; } -step read_part { SELECT * FROM parttbl ORDER BY a; } +step read_part { SELECT * FROM parttbl ORDER BY a, c; } # this test exercises EvalPlanQual with a CTE, cf bug #14328 step readwcte { @@ -345,6 +356,8 @@ permutation upsert1 upsert2 c1 c2 read permutation readp1 writep1 readp2 c1 c2 permutation writep2 returningp1 c1 c2 permutation writep3a writep3b c1 c2 +permutation writep4a writep4b c1 c2 readp +permutation writep4a deletep4 c1 c2 readp permutation wx2 partiallock c2 c1 read permutation wx2 lockwithvalues c2 c1 read permutation wx2_ext partiallock_ext c2 c1 read_ext @@ -356,6 +369,7 @@ permutation wrjt selectjoinforupdate c2 c1 permutation wrjt selectresultforupdate c2 c1 permutation wrtwcte multireadwcte c1 c2 +permutation simplepartupdate conditionalpartupdate c1 c2 read_part permutation simplepartupdate complexpartupdate c1 c2 read_part permutation simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2 read_part permutation simplepartupdate_noroute complexpartupdate_route c1 c2 read_part