From e46911bd7bb621b66c8d693132d4aeb3202d73df Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 19 Jul 2018 16:14:51 +0900 Subject: [PATCH v2 1/4] Don't lock range table relations in the executor As noted in https://postgre.es/m/4114.1531674142%40sss.pgh.pa.us, taking relation locks inside the executor proper is redundant, because appropriate locks should've been taken by the upstream code, that is, during the parse/rewrite/plan pipeline when adding the relations to range table or by the AcquireExecutorLocks if using a cached plan. Also, InitPlan would do certain initiaization steps, such as creating result rels, ExecRowMarks, etc. only because of the concerns about locking order, which it needs not to do anymore, because we've eliminated executor locking at all. Instead create result rels and ExecRowMarks, etc. in the ExecInit* routines of the respective nodes. To indicate which lock was taken on a relation before creating a RTE_RELATION range table entry for it, add a 'lockmode' field to RangeTblEntry. It's set when the relation is opened for the first time in the parse/rewrite/plan pipeline and its range table entry created. --- src/backend/commands/view.c | 2 + src/backend/executor/execMain.c | 269 ++++++++++++--------------------- src/backend/executor/execPartition.c | 4 +- src/backend/executor/execUtils.c | 24 +-- src/backend/executor/nodeLockRows.c | 2 +- src/backend/executor/nodeModifyTable.c | 39 ++++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/parser/analyze.c | 1 + src/backend/parser/parse_clause.c | 1 + src/backend/parser/parse_relation.c | 1 + src/backend/parser/parse_utilcmd.c | 4 + src/backend/rewrite/rewriteHandler.c | 18 +-- src/backend/storage/lmgr/lmgr.c | 7 +- src/backend/utils/cache/plancache.c | 26 +--- src/include/executor/executor.h | 2 +- src/include/nodes/parsenodes.h | 1 + src/include/storage/lmgr.h | 2 +- 20 files changed, 166 insertions(+), 241 deletions(-) diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index ffb71c0ea7..818557d796 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -388,9 +388,11 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse) rt_entry1 = addRangeTableEntryForRelation(pstate, viewRel, makeAlias("old", NIL), false, false); + rt_entry1->lockmode = AccessShareLock; rt_entry2 = addRangeTableEntryForRelation(pstate, viewRel, makeAlias("new", NIL), false, false); + rt_entry2->lockmode = AccessShareLock; /* Must override addRangeTableEntry's default access-check flags */ rt_entry1->requiredPerms = 0; rt_entry2->requiredPerms = 0; diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index c583e020a0..298bf43fce 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -50,10 +50,12 @@ #include "mb/pg_wchar.h" #include "miscadmin.h" #include "optimizer/clauses.h" +#include "optimizer/prep.h" #include "parser/parsetree.h" #include "rewrite/rewriteManip.h" #include "storage/bufmgr.h" #include "storage/lmgr.h" +#include "storage/lockdefs.h" #include "tcop/utility.h" #include "utils/acl.h" #include "utils/lsyscache.h" @@ -818,6 +820,26 @@ InitPlan(QueryDesc *queryDesc, int eflags) */ ExecCheckRTPerms(rangeTable, true); +#ifdef USE_ASSERT_CHECKING + foreach(l, rangeTable) + { + RangeTblEntry *rte = lfirst(l); + + if (rte->rtekind == RTE_RELATION && OidIsValid(rte->relid)) + { + /* + * The following asserts that no new lock needed to be taken, + * meaning the upstream code already acquired the needed locks. + */ + Assert(rte->lockmode != NoLock); + if (LockRelationOid(rte->relid, rte->lockmode) && + !IsParallelWorker()) + elog(NOTICE, "InitPlan: lock on \"%s\" not already taken", + get_rel_name(rte->relid)); + } + } +#endif + /* * initialize the node's execution state */ @@ -832,85 +854,19 @@ InitPlan(QueryDesc *queryDesc, int eflags) */ if (plannedstmt->resultRelations) { - List *resultRelations = plannedstmt->resultRelations; - int numResultRelations = list_length(resultRelations); - ResultRelInfo *resultRelInfos; - ResultRelInfo *resultRelInfo; + int numResultRelations = list_length(plannedstmt->resultRelations); + int num_roots = list_length(plannedstmt->rootResultRelations); - resultRelInfos = (ResultRelInfo *) - palloc(numResultRelations * sizeof(ResultRelInfo)); - resultRelInfo = resultRelInfos; - foreach(l, resultRelations) - { - Index resultRelationIndex = lfirst_int(l); - Oid resultRelationOid; - Relation resultRelation; - - resultRelationOid = getrelid(resultRelationIndex, rangeTable); - resultRelation = heap_open(resultRelationOid, RowExclusiveLock); - - InitResultRelInfo(resultRelInfo, - resultRelation, - resultRelationIndex, - NULL, - estate->es_instrument); - resultRelInfo++; - } - estate->es_result_relations = resultRelInfos; + estate->es_result_relations = (ResultRelInfo *) + palloc(numResultRelations * sizeof(ResultRelInfo)); estate->es_num_result_relations = numResultRelations; /* es_result_relation_info is NULL except when within ModifyTable */ estate->es_result_relation_info = NULL; - /* - * In the partitioned result relation case, lock the non-leaf result - * relations too. A subset of these are the roots of respective - * partitioned tables, for which we also allocate ResultRelInfos. - */ - estate->es_root_result_relations = NULL; - estate->es_num_root_result_relations = 0; - if (plannedstmt->nonleafResultRelations) - { - int num_roots = list_length(plannedstmt->rootResultRelations); - - /* - * Firstly, build ResultRelInfos for all the partitioned table - * roots, because we will need them to fire the statement-level - * triggers, if any. - */ - resultRelInfos = (ResultRelInfo *) + /* For partitioned tables that are roots of their partition trees. */ + estate->es_root_result_relations = (ResultRelInfo *) palloc(num_roots * sizeof(ResultRelInfo)); - resultRelInfo = resultRelInfos; - foreach(l, plannedstmt->rootResultRelations) - { - Index resultRelIndex = lfirst_int(l); - Oid resultRelOid; - Relation resultRelDesc; - - resultRelOid = getrelid(resultRelIndex, rangeTable); - resultRelDesc = heap_open(resultRelOid, RowExclusiveLock); - InitResultRelInfo(resultRelInfo, - resultRelDesc, - lfirst_int(l), - NULL, - estate->es_instrument); - resultRelInfo++; - } - - estate->es_root_result_relations = resultRelInfos; - estate->es_num_root_result_relations = num_roots; - - /* Simply lock the rest of them. */ - foreach(l, plannedstmt->nonleafResultRelations) - { - Index resultRelIndex = lfirst_int(l); - - /* We locked the roots above. */ - if (!list_member_int(plannedstmt->rootResultRelations, - resultRelIndex)) - LockRelationOid(getrelid(resultRelIndex, rangeTable), - RowExclusiveLock); - } - } + estate->es_num_root_result_relations = num_roots; } else { @@ -924,73 +880,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_num_root_result_relations = 0; } - /* - * Similarly, we have to lock relations selected FOR [KEY] UPDATE/SHARE - * before we initialize the plan tree, else we'd be risking lock upgrades. - * While we are at it, build the ExecRowMark list. Any partitioned child - * tables are ignored here (because isParent=true) and will be locked by - * the first Append or MergeAppend node that references them. (Note that - * the RowMarks corresponding to partitioned child tables are present in - * the same list as the rest, i.e., plannedstmt->rowMarks.) - */ estate->es_rowMarks = NIL; - foreach(l, plannedstmt->rowMarks) - { - PlanRowMark *rc = (PlanRowMark *) lfirst(l); - Oid relid; - Relation relation; - ExecRowMark *erm; - - /* ignore "parent" rowmarks; they are irrelevant at runtime */ - if (rc->isParent) - continue; - - /* get relation's OID (will produce InvalidOid if subquery) */ - relid = getrelid(rc->rti, rangeTable); - - /* - * If you change the conditions under which rel locks are acquired - * here, be sure to adjust ExecOpenScanRelation to match. - */ - switch (rc->markType) - { - case ROW_MARK_EXCLUSIVE: - case ROW_MARK_NOKEYEXCLUSIVE: - case ROW_MARK_SHARE: - case ROW_MARK_KEYSHARE: - relation = heap_open(relid, RowShareLock); - break; - case ROW_MARK_REFERENCE: - relation = heap_open(relid, AccessShareLock); - break; - case ROW_MARK_COPY: - /* no physical table access is required */ - relation = NULL; - break; - default: - elog(ERROR, "unrecognized markType: %d", rc->markType); - relation = NULL; /* keep compiler quiet */ - break; - } - - /* Check that relation is a legal target for marking */ - if (relation) - CheckValidRowMarkRel(relation, rc->markType); - - erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); - erm->relation = relation; - erm->relid = relid; - erm->rti = rc->rti; - erm->prti = rc->prti; - erm->rowmarkId = rc->rowmarkId; - erm->markType = rc->markType; - erm->strength = rc->strength; - erm->waitPolicy = rc->waitPolicy; - erm->ermActive = false; - ItemPointerSetInvalid(&(erm->curCtid)); - erm->ermExtra = NULL; - estate->es_rowMarks = lappend(estate->es_rowMarks, erm); - } /* * Initialize the executor's tuple table to empty. @@ -1597,8 +1487,6 @@ ExecPostprocessPlan(EState *estate) static void ExecEndPlan(PlanState *planstate, EState *estate) { - ResultRelInfo *resultRelInfo; - int i; ListCell *l; /* @@ -1624,26 +1512,6 @@ ExecEndPlan(PlanState *planstate, EState *estate) */ ExecResetTupleTable(estate->es_tupleTable, false); - /* - * close the result relation(s) if any, but hold locks until xact commit. - */ - resultRelInfo = estate->es_result_relations; - for (i = estate->es_num_result_relations; i > 0; i--) - { - /* Close indices and then the relation itself */ - ExecCloseIndices(resultRelInfo); - heap_close(resultRelInfo->ri_RelationDesc, NoLock); - resultRelInfo++; - } - - /* Close the root target relation(s). */ - resultRelInfo = estate->es_root_result_relations; - for (i = estate->es_num_root_result_relations; i > 0; i--) - { - heap_close(resultRelInfo->ri_RelationDesc, NoLock); - resultRelInfo++; - } - /* likewise close any trigger target relations */ ExecCleanUpTriggerState(estate); @@ -2404,25 +2272,64 @@ ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo) } /* - * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index - * - * If no such struct, either return NULL or throw error depending on missing_ok + * ExecBuildRowMark -- create ExecRowMark struct for given PlanRowMark */ ExecRowMark * -ExecFindRowMark(EState *estate, Index rti, bool missing_ok) +ExecBuildRowMark(EState *estate, PlanRowMark *rc) { - ListCell *lc; + ExecRowMark *erm; + Oid relid; + Relation relation; - foreach(lc, estate->es_rowMarks) + Assert(!rc->isParent); + + /* get relation's OID (will produce InvalidOid if subquery) */ + relid = getrelid(rc->rti, estate->es_range_table); + + /* + * If you change the conditions under which rel locks are acquired + * here, be sure to adjust ExecOpenScanRelation to match. + */ + switch (rc->markType) { - ExecRowMark *erm = (ExecRowMark *) lfirst(lc); - - if (erm->rti == rti) - return erm; + case ROW_MARK_EXCLUSIVE: + case ROW_MARK_NOKEYEXCLUSIVE: + case ROW_MARK_SHARE: + case ROW_MARK_KEYSHARE: + relation = heap_open(relid, NoLock); + break; + case ROW_MARK_REFERENCE: + relation = heap_open(relid, NoLock); + break; + case ROW_MARK_COPY: + /* no physical table access is required */ + relation = NULL; + break; + default: + elog(ERROR, "unrecognized markType: %d", rc->markType); + relation = NULL; /* keep compiler quiet */ + break; } - if (!missing_ok) - elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti); - return NULL; + + /* Check that relation is a legal target for marking */ + if (relation) + CheckValidRowMarkRel(relation, rc->markType); + + erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); + erm->relation = relation; + erm->relid = relid; + erm->rti = rc->rti; + erm->prti = rc->prti; + erm->rowmarkId = rc->rowmarkId; + erm->markType = rc->markType; + erm->strength = rc->strength; + erm->waitPolicy = rc->waitPolicy; + erm->ermActive = false; + ItemPointerSetInvalid(&(erm->curCtid)); + erm->ermExtra = NULL; + estate->es_rowMarks = lappend(estate->es_rowMarks, erm); + + return erm; } /* @@ -3154,7 +3061,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) } /* es_result_relation_info must NOT be copied */ /* es_trig_target_relations must NOT be copied */ - estate->es_rowMarks = parentestate->es_rowMarks; + /* es_rowMarks must NOT be copied */ estate->es_top_eflags = parentestate->es_top_eflags; estate->es_instrument = parentestate->es_instrument; /* es_auxmodifytables must NOT be copied */ @@ -3267,6 +3174,18 @@ EvalPlanQualEnd(EPQState *epqstate) ExecEndNode(subplanstate); } + /* + * close any relations selected FOR [KEY] UPDATE/SHARE, again keeping + * locks + */ + foreach(l, estate->es_rowMarks) + { + ExecRowMark *erm = (ExecRowMark *) lfirst(l); + + if (erm->relation) + heap_close(erm->relation, NoLock); + } + /* throw away the per-estate tuple table */ ExecResetTupleTable(estate->es_tupleTable, false); diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 1a9943c3aa..81c2f5cedc 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -1510,9 +1510,7 @@ ExecCreatePartitionPruneState(PlanState *planstate, /* * We need to hold a pin on the partitioned table's relcache entry * so that we can rely on its copies of the table's partition key - * and partition descriptor. We need not get a lock though; one - * should have been acquired already by InitPlan or - * ExecLockNonLeafAppendTables. + * and partition descriptor. */ context->partrel = relation_open(pinfo->reloid, NoLock); diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 5b3eaec80b..09942b34fb 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -42,6 +42,7 @@ #include "postgres.h" +#include "access/parallel.h" #include "access/relscan.h" #include "access/transam.h" #include "executor/executor.h" @@ -51,6 +52,7 @@ #include "parser/parsetree.h" #include "storage/lmgr.h" #include "utils/builtins.h" +#include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/rel.h" #include "utils/typcache.h" @@ -652,28 +654,10 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags) { Relation rel; Oid reloid; - LOCKMODE lockmode; - /* - * Determine the lock type we need. First, scan to see if target relation - * is a result relation. If not, check if it's a FOR UPDATE/FOR SHARE - * relation. In either of those cases, we got the lock already. - */ - lockmode = AccessShareLock; - if (ExecRelationIsTargetRelation(estate, scanrelid)) - lockmode = NoLock; - else - { - /* Keep this check in sync with InitPlan! */ - ExecRowMark *erm = ExecFindRowMark(estate, scanrelid, true); - - if (erm != NULL && erm->relation != NULL) - lockmode = NoLock; - } - - /* Open the relation and acquire lock as needed */ + /* Open the relation. */ reloid = getrelid(scanrelid, estate->es_range_table); - rel = heap_open(reloid, lockmode); + rel = heap_open(reloid, NoLock); /* * Complain if we're attempting a scan of an unscannable relation, except diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 30de8a95ab..8c80291a53 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -425,7 +425,7 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags) Assert(rc->rti > 0 && rc->rti <= lrstate->lr_ntables); /* find ExecRowMark and build ExecAuxRowMark */ - erm = ExecFindRowMark(estate, rc->rti, false); + erm = ExecBuildRowMark(estate, rc); aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist); /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index d8d89c7983..93e5bf7af6 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -46,6 +46,7 @@ #include "foreign/fdwapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" +#include "parser/parsetree.h" #include "storage/bufmgr.h" #include "storage/lmgr.h" #include "utils/builtins.h" @@ -2238,12 +2239,36 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans); mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex; + resultRelInfo = mtstate->resultRelInfo; + foreach(l, node->resultRelations) + { + Index resultRelationIndex = lfirst_int(l); + RangeTblEntry *rte = rt_fetch(resultRelationIndex, + estate->es_range_table); + Relation rel = heap_open(rte->relid, NoLock); + + InitResultRelInfo(resultRelInfo, rel, resultRelationIndex, NULL, + estate->es_instrument); + resultRelInfo++; + } /* If modifying a partitioned table, initialize the root table info */ if (node->rootResultRelIndex >= 0) + { + Index root_rt_index = linitial_int(node->partitioned_rels); + RangeTblEntry *rte; + Relation rel; + mtstate->rootResultRelInfo = estate->es_root_result_relations + node->rootResultRelIndex; + Assert(root_rt_index > 0); + rte = rt_fetch(root_rt_index, estate->es_range_table); + rel = heap_open(rte->relid, NoLock); + InitResultRelInfo(mtstate->rootResultRelInfo, rel, root_rt_index, + NULL, estate->es_instrument); + } + mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans); mtstate->mt_nplans = nplans; @@ -2530,7 +2555,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) continue; /* find ExecRowMark (same for all subplans) */ - erm = ExecFindRowMark(estate, rc->rti, false); + erm = ExecBuildRowMark(estate, rc); /* build ExecAuxRowMark for each subplan */ for (i = 0; i < nplans; i++) @@ -2687,19 +2712,29 @@ ExecEndModifyTable(ModifyTableState *node) int i; /* - * Allow any FDWs to shut down + * close the result relation(s) if any, but hold locks until xact commit. */ for (i = 0; i < node->mt_nplans; i++) { ResultRelInfo *resultRelInfo = node->resultRelInfo + i; + /* Allow any FDWs to shut down. */ if (!resultRelInfo->ri_usesFdwDirectModify && resultRelInfo->ri_FdwRoutine != NULL && resultRelInfo->ri_FdwRoutine->EndForeignModify != NULL) resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state, resultRelInfo); + + /* Close indices and then the relation itself */ + ExecCloseIndices(resultRelInfo); + heap_close(resultRelInfo->ri_RelationDesc, NoLock); + resultRelInfo++; } + /* Close the root partitioned table, if any. */ + if (node->rootResultRelInfo) + heap_close(node->rootResultRelInfo->ri_RelationDesc, NoLock); + /* Close all the partitioned tables, leaf partitions, and their indices */ if (node->mt_partition_tuple_routing) ExecCleanupTupleRouting(node, node->mt_partition_tuple_routing); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 7c8220cf65..6d685db0ea 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2356,6 +2356,7 @@ _copyRangeTblEntry(const RangeTblEntry *from) COPY_SCALAR_FIELD(rtekind); COPY_SCALAR_FIELD(relid); COPY_SCALAR_FIELD(relkind); + COPY_SCALAR_FIELD(lockmode); COPY_NODE_FIELD(tablesample); COPY_NODE_FIELD(subquery); COPY_SCALAR_FIELD(security_barrier); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 378f2facb8..488fddb255 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2630,6 +2630,7 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b) COMPARE_SCALAR_FIELD(rtekind); COMPARE_SCALAR_FIELD(relid); COMPARE_SCALAR_FIELD(relkind); + COMPARE_SCALAR_FIELD(lockmode); COMPARE_NODE_FIELD(tablesample); COMPARE_NODE_FIELD(subquery); COMPARE_SCALAR_FIELD(security_barrier); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index b5af904c18..53f209e170 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -3127,6 +3127,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node) case RTE_RELATION: WRITE_OID_FIELD(relid); WRITE_CHAR_FIELD(relkind); + WRITE_INT_FIELD(lockmode); WRITE_NODE_FIELD(tablesample); break; case RTE_SUBQUERY: diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 3254524223..7a1e839ef4 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1350,6 +1350,7 @@ _readRangeTblEntry(void) case RTE_RELATION: READ_OID_FIELD(relid); READ_CHAR_FIELD(relkind); + READ_INT_FIELD(lockmode); READ_NODE_FIELD(tablesample); break; case RTE_SUBQUERY: diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index c601b6d40d..f70597cbd3 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1040,6 +1040,7 @@ transformOnConflictClause(ParseState *pstate, makeAlias("excluded", NIL), false, false); exclRte->relkind = RELKIND_COMPOSITE_TYPE; + exclRte->lockmode = RowExclusiveLock; exclRte->requiredPerms = 0; /* other permissions fields in exclRte are already empty */ diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index cfd4b91897..b6a1b60d1e 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -218,6 +218,7 @@ setTargetTable(ParseState *pstate, RangeVar *relation, */ rte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation, relation->alias, inh, false); + rte->lockmode = RowExclusiveLock; pstate->p_target_rangetblentry = rte; /* assume new rte is at end */ diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index bf5df26009..3fd91bdbb3 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1217,6 +1217,7 @@ addRangeTableEntry(ParseState *pstate, rel = parserOpenTable(pstate, relation, lockmode); rte->relid = RelationGetRelid(rel); rte->relkind = rel->rd_rel->relkind; + rte->lockmode = lockmode; /* * Build the list of effective column names using user-supplied aliases diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 656b1b5f1b..54c6b1e082 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -2636,9 +2636,11 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, oldrte = addRangeTableEntryForRelation(pstate, rel, makeAlias("old", NIL), false, false); + oldrte->lockmode = AccessShareLock; newrte = addRangeTableEntryForRelation(pstate, rel, makeAlias("new", NIL), false, false); + newrte->lockmode = AccessShareLock; /* Must override addRangeTableEntry's default access-check flags */ oldrte->requiredPerms = 0; newrte->requiredPerms = 0; @@ -2734,9 +2736,11 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, oldrte = addRangeTableEntryForRelation(sub_pstate, rel, makeAlias("old", NIL), false, false); + oldrte->lockmode = AccessShareLock; newrte = addRangeTableEntryForRelation(sub_pstate, rel, makeAlias("new", NIL), false, false); + newrte->lockmode = AccessShareLock; oldrte->requiredPerms = 0; newrte->requiredPerms = 0; addRTEtoQuery(sub_pstate, oldrte, false, true, false); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index d830569641..f99c15bfcd 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -147,7 +147,6 @@ AcquireRewriteLocks(Query *parsetree, { RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); Relation rel; - LOCKMODE lockmode; List *newaliasvars; Index curinputvarno; RangeTblEntry *curinputrte; @@ -162,21 +161,8 @@ AcquireRewriteLocks(Query *parsetree, * Grab the appropriate lock type for the relation, and do not * release it until end of transaction. This protects the * rewriter and planner against schema changes mid-query. - * - * Assuming forExecute is true, this logic must match what the - * executor will do, else we risk lock-upgrade deadlocks. */ - if (!forExecute) - lockmode = AccessShareLock; - else if (rt_index == parsetree->resultRelation) - lockmode = RowExclusiveLock; - else if (forUpdatePushedDown || - get_parse_rowmark(parsetree, rt_index) != NULL) - lockmode = RowShareLock; - else - lockmode = AccessShareLock; - - rel = heap_open(rte->relid, lockmode); + rel = heap_open(rte->relid, rte->lockmode); /* * While we have the relation open, update the RTE's relkind, @@ -2895,6 +2881,7 @@ rewriteTargetView(Query *parsetree, Relation view) * it changed since this view was made (cf. AcquireRewriteLocks). */ base_rte->relkind = base_rel->rd_rel->relkind; + base_rte->lockmode = RowExclusiveLock; /* * If the view query contains any sublink subqueries then we need to also @@ -3102,6 +3089,7 @@ rewriteTargetView(Query *parsetree, Relation view) NIL), false, false); new_exclRte->relkind = RELKIND_COMPOSITE_TYPE; + new_exclRte->lockmode = RowExclusiveLock; new_exclRte->requiredPerms = 0; /* other permissions fields in new_exclRte are already empty */ diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index 7b2dcb6c60..a1ebf647de 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -100,8 +100,9 @@ SetLocktagRelationOid(LOCKTAG *tag, Oid relid) * * Lock a relation given only its OID. This should generally be used * before attempting to open the relation's relcache entry. + * Return TRUE if we acquired a new lock, FALSE if already held. */ -void +bool LockRelationOid(Oid relid, LOCKMODE lockmode) { LOCKTAG tag; @@ -122,7 +123,11 @@ LockRelationOid(Oid relid, LOCKMODE lockmode) * CommandCounterIncrement, not here.) */ if (res != LOCKACQUIRE_ALREADY_HELD) + { AcceptInvalidationMessages(); + return true; + } + return false; } /* diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 7271b5880b..b57e4d81ad 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -1516,8 +1516,6 @@ AcquireExecutorLocks(List *stmt_list, bool acquire) foreach(lc2, plannedstmt->rtable) { RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc2); - LOCKMODE lockmode; - PlanRowMark *rc; rt_index++; @@ -1530,19 +1528,10 @@ AcquireExecutorLocks(List *stmt_list, bool acquire) * fail if it's been dropped entirely --- we'll just transiently * acquire a non-conflicting lock. */ - if (list_member_int(plannedstmt->resultRelations, rt_index) || - list_member_int(plannedstmt->nonleafResultRelations, rt_index)) - lockmode = RowExclusiveLock; - else if ((rc = get_plan_rowmark(plannedstmt->rowMarks, rt_index)) != NULL && - RowMarkRequiresRowShareLock(rc->markType)) - lockmode = RowShareLock; - else - lockmode = AccessShareLock; - if (acquire) - LockRelationOid(rte->relid, lockmode); + LockRelationOid(rte->relid, rte->lockmode); else - UnlockRelationOid(rte->relid, lockmode); + UnlockRelationOid(rte->relid, rte->lockmode); } } } @@ -1596,23 +1585,16 @@ ScanQueryForLocks(Query *parsetree, bool acquire) foreach(lc, parsetree->rtable) { RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc); - LOCKMODE lockmode; rt_index++; switch (rte->rtekind) { case RTE_RELATION: /* Acquire or release the appropriate type of lock */ - if (rt_index == parsetree->resultRelation) - lockmode = RowExclusiveLock; - else if (get_parse_rowmark(parsetree, rt_index) != NULL) - lockmode = RowShareLock; - else - lockmode = AccessShareLock; if (acquire) - LockRelationOid(rte->relid, lockmode); + LockRelationOid(rte->relid, rte->lockmode); else - UnlockRelationOid(rte->relid, lockmode); + UnlockRelationOid(rte->relid, rte->lockmode); break; case RTE_SUBQUERY: diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index f82b51667f..4425b978f9 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -188,7 +188,7 @@ extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo); -extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok); +extern ExecRowMark *ExecBuildRowMark(EState *estate, PlanRowMark *rc); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, Relation relation, Index rti, int lockmode, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 07ab1a3dde..e8ac2459d1 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -977,6 +977,7 @@ typedef struct RangeTblEntry */ Oid relid; /* OID of the relation */ char relkind; /* relation kind (see pg_class.relkind) */ + int lockmode; /* lock taken on the relation or 0 */ struct TableSampleClause *tablesample; /* sampling info, or NULL */ /* diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h index a217de9716..69e6f7ffd6 100644 --- a/src/include/storage/lmgr.h +++ b/src/include/storage/lmgr.h @@ -37,7 +37,7 @@ typedef enum XLTW_Oper extern void RelationInitLockInfo(Relation relation); /* Lock a relation */ -extern void LockRelationOid(Oid relid, LOCKMODE lockmode); +extern bool LockRelationOid(Oid relid, LOCKMODE lockmode); extern bool ConditionalLockRelationOid(Oid relid, LOCKMODE lockmode); extern void UnlockRelationId(LockRelId *relid, LOCKMODE lockmode); extern void UnlockRelationOid(Oid relid, LOCKMODE lockmode); -- 2.11.0