From 636349f40265854b318ccf46700ec57731db8793 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 3 Dec 2025 15:07:24 -0500 Subject: [PATCH v43 06/10] Track which relations are modified by a query Save the relids of modified relations in a bitmap in the PlannedStmt. A later commit will pass this information down to scan nodes to control whether or not on-access pruning is allowed to set the visibility map. Setting the visibility map during a scan is counterproductive if the query is going to modify the page immediately after. Relations are considered modified if they are the target of INSERT, UPDATE, DELETE, or MERGE, or if they have any row mark (including SELECT FOR UPDATE/SHARE). All row mark types are included, even those which don't actually modify tuples, because this bitmap is only used as a hint to avoid unnecessary work. Author: Melanie Plageman Reviewed-by: Andres Freund Reviewed-by: Chao Li Discussion: https://postgr.es/m/F5CDD1B5-628C-44A1-9F85-3958C626F6A9%40gmail.com --- src/backend/executor/execMain.c | 47 ++++++++++++++++++++++++++ src/backend/executor/execParallel.c | 1 + src/backend/executor/nodeLockRows.c | 4 +++ src/backend/executor/nodeModifyTable.c | 18 ++++++++++ src/backend/optimizer/plan/planner.c | 21 +++++++++++- src/include/nodes/plannodes.h | 10 ++++++ 6 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 58b84955c2b..3f134f9a34d 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -90,6 +90,9 @@ static bool ExecCheckPermissionsModified(Oid relOid, Oid userid, Bitmapset *modifiedCols, AclMode requiredPerms); static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt); +#ifdef USE_ASSERT_CHECKING +static void ExecCheckModifiedRelIds(EState *estate); +#endif static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree); static void ReportNotNullViolationError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, @@ -827,6 +830,46 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt) } +/* + * ExecCheckModifiedRelIds + * Verify that every relation the executor actually opened for modification + * or row locking is present in the planner's modifiedRelids. + * + * The planner's set may be a superset of what the executor touches, because it + * includes partitions that were pruned at runtime and parent row marks that the + * executor skips. + */ +#ifdef USE_ASSERT_CHECKING +static void +ExecCheckModifiedRelIds(EState *estate) +{ + PlannedStmt *plannedstmt = estate->es_plannedstmt; + Bitmapset *executor_relids = NULL; + ListCell *lc; + + foreach(lc, estate->es_opened_result_relations) + { + ResultRelInfo *rri = (ResultRelInfo *) lfirst(lc); + + if (rri->ri_RangeTableIndex != 0) + executor_relids = bms_add_member(executor_relids, + rri->ri_RangeTableIndex); + } + if (estate->es_rowmarks) + { + for (int i = 0; i < estate->es_range_table_size; i++) + { + if (estate->es_rowmarks[i] != NULL) + executor_relids = bms_add_member(executor_relids, + estate->es_rowmarks[i]->rti); + } + } + Assert(bms_is_subset(executor_relids, plannedstmt->modifiedRelids)); + bms_free(executor_relids); +} +#endif + + /* ---------------------------------------------------------------- * InitPlan * @@ -992,6 +1035,10 @@ InitPlan(QueryDesc *queryDesc, int eflags) */ planstate = ExecInitNode(plan, estate, eflags); +#ifdef USE_ASSERT_CHECKING + ExecCheckModifiedRelIds(estate); +#endif + /* * Get the tuple descriptor describing the type of tuples to return. */ diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index ac84af294c9..4f39767d033 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -188,6 +188,7 @@ ExecSerializePlan(Plan *plan, EState *estate) pstmt->partPruneInfos = estate->es_part_prune_infos; pstmt->rtable = estate->es_range_table; pstmt->unprunableRelids = estate->es_unpruned_relids; + pstmt->modifiedRelids = estate->es_plannedstmt->modifiedRelids; pstmt->permInfos = estate->es_rteperminfos; pstmt->resultRelations = NIL; pstmt->appendRelations = NIL; diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 8d865470780..d67f24fca8c 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -113,6 +113,10 @@ lnext: } erm->ermActive = true; + /* verify this relation is in the planner's modifiedRelids */ + Assert(bms_is_member(erm->rti, + estate->es_plannedstmt->modifiedRelids)); + /* fetch the tuple's ctid */ datum = ExecGetJunkAttribute(slot, aerm->ctidAttNo, diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 4cd5e262e0f..6b4ee4f9378 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -896,6 +896,16 @@ ExecInsert(ModifyTableContext *context, resultRelationDesc = resultRelInfo->ri_RelationDesc; + /* + * Verify this relation is in the planner's set of modified relations. + * Partitions opened by tuple routing have ri_RangeTableIndex == 0 because + * they have no range table entry, so we can only check relations that are + * in the range table. + */ + Assert(resultRelInfo->ri_RangeTableIndex == 0 || + bms_is_member(resultRelInfo->ri_RangeTableIndex, + estate->es_plannedstmt->modifiedRelids)); + /* * Open the table's indexes, if we have not done so already, so that we * can add new index entries for the inserted tuple. @@ -1523,6 +1533,10 @@ ExecDeleteAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo, { EState *estate = context->estate; + Assert(resultRelInfo->ri_RangeTableIndex == 0 || + bms_is_member(resultRelInfo->ri_RangeTableIndex, + estate->es_plannedstmt->modifiedRelids)); + return table_tuple_delete(resultRelInfo->ri_RelationDesc, tupleid, estate->es_output_cid, estate->es_snapshot, @@ -2205,6 +2219,10 @@ ExecUpdateAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo, bool partition_constraint_failed; TM_Result result; + Assert(resultRelInfo->ri_RangeTableIndex == 0 || + bms_is_member(resultRelInfo->ri_RangeTableIndex, + estate->es_plannedstmt->modifiedRelids)); + updateCxt->crossPartUpdate = false; /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 42604a0f75c..847af979e31 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -340,8 +340,10 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, RelOptInfo *final_rel; Path *best_path; Plan *top_plan; + Bitmapset *modifiedRelids = NULL; ListCell *lp, - *lr; + *lr, + *lc; /* * Set up global state for this planner invocation. This data is needed @@ -661,6 +663,23 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, result->subplans = glob->subplans; result->rewindPlanIDs = glob->rewindPlanIDs; result->rowMarks = glob->finalrowmarks; + + /* + * Compute modifiedRelids from result relations and row marks. This is a + * superset of what the executor will actually modify/lock at runtime, + * because runtime partition pruning may eliminate some result relations, + * and parent row marks are included here but skipped by the executor. + */ + foreach(lc, glob->resultRelations) + modifiedRelids = bms_add_member(modifiedRelids, lfirst_int(lc)); + foreach(lc, glob->finalrowmarks) + { + PlanRowMark *rc = (PlanRowMark *) lfirst(lc); + + modifiedRelids = bms_add_member(modifiedRelids, rc->rti); + } + result->modifiedRelids = modifiedRelids; + result->relationOids = glob->relationOids; result->invalItems = glob->invalItems; result->paramExecTypes = glob->paramExecTypes; diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index b6185825fcb..841c7707c59 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -112,6 +112,16 @@ typedef struct PlannedStmt */ Bitmapset *unprunableRelids; + /* + * RT indexes of relations modified by the query through + * UPDATE/DELETE/INSERT/MERGE or targeted by SELECT FOR UPDATE/SHARE. + * + * Computed by the planner, this is a superset of what the executor will + * actually touch at runtime, because it includes partitions that may be + * pruned and parent row marks that the executor skips. + */ + Bitmapset *modifiedRelids; + /* * list of RTEPermissionInfo nodes for rtable entries needing one */ -- 2.43.0