diff options
29 files changed, 159 insertions, 38 deletions
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 4f1d3653575..7dfa3278a5b 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1415,6 +1415,7 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender, rte.rtekind = RTE_RELATION; rte.relid = relId; rte.relkind = RELKIND_RELATION; /* no need for exactness here */ + rte.rellockmode = AccessShareLock; context.rtables = list_make1(list_make1(&rte)); diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 9176f6280b0..4ad58689fc3 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2549,6 +2549,7 @@ AddRelationNewConstraints(Relation rel, pstate->p_sourcetext = queryString; rte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, NULL, false, true); diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index d06662bd32e..2d5bc8add68 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -833,20 +833,21 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, if (stmt->relation) { + LOCKMODE lockmode = is_from ? RowExclusiveLock : AccessShareLock; + RangeTblEntry *rte; TupleDesc tupDesc; List *attnums; ListCell *cur; - RangeTblEntry *rte; Assert(!stmt->query); /* Open and lock the relation, using the appropriate lock type. */ - rel = heap_openrv(stmt->relation, - (is_from ? RowExclusiveLock : AccessShareLock)); + rel = heap_openrv(stmt->relation, lockmode); relid = RelationGetRelid(rel); - rte = addRangeTableEntryForRelation(pstate, rel, NULL, false, false); + rte = addRangeTableEntryForRelation(pstate, rel, lockmode, + NULL, false, false); rte->requiredPerms = (is_from ? ACL_INSERT : ACL_SELECT); tupDesc = RelationGetDescr(rel); diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 3d82edbf581..d5cb62da15b 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -528,6 +528,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) rte->rtekind = RTE_RELATION; rte->relid = intoRelationAddr.objectId; rte->relkind = relkind; + rte->rellockmode = RowExclusiveLock; rte->requiredPerms = ACL_INSERT; for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++) diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index cee0ef915be..2fd17b24b9e 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -567,7 +567,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id) qual_expr = stringToNode(qual_value); /* Add this rel to the parsestate's rangetable, for dependencies */ - addRangeTableEntryForRelation(qual_pstate, rel, NULL, false, false); + addRangeTableEntryForRelation(qual_pstate, rel, + AccessShareLock, + NULL, false, false); qual_parse_rtable = qual_pstate->p_rtable; free_parsestate(qual_pstate); @@ -589,8 +591,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id) with_check_qual = stringToNode(with_check_value); /* Add this rel to the parsestate's rangetable, for dependencies */ - addRangeTableEntryForRelation(with_check_pstate, rel, NULL, false, - false); + addRangeTableEntryForRelation(with_check_pstate, rel, + AccessShareLock, + NULL, false, false); with_check_parse_rtable = with_check_pstate->p_rtable; free_parsestate(with_check_pstate); @@ -752,11 +755,13 @@ CreatePolicy(CreatePolicyStmt *stmt) /* Add for the regular security quals */ rte = addRangeTableEntryForRelation(qual_pstate, target_table, + AccessShareLock, NULL, false, false); addRTEtoQuery(qual_pstate, rte, false, true, true); /* Add for the with-check quals */ rte = addRangeTableEntryForRelation(with_check_pstate, target_table, + AccessShareLock, NULL, false, false); addRTEtoQuery(with_check_pstate, rte, false, true, true); @@ -928,6 +933,7 @@ AlterPolicy(AlterPolicyStmt *stmt) ParseState *qual_pstate = make_parsestate(NULL); rte = addRangeTableEntryForRelation(qual_pstate, target_table, + AccessShareLock, NULL, false, false); addRTEtoQuery(qual_pstate, rte, false, true, true); @@ -950,6 +956,7 @@ AlterPolicy(AlterPolicyStmt *stmt) ParseState *with_check_pstate = make_parsestate(NULL); rte = addRangeTableEntryForRelation(with_check_pstate, target_table, + AccessShareLock, NULL, false, false); addRTEtoQuery(with_check_pstate, rte, false, true, true); @@ -1096,8 +1103,9 @@ AlterPolicy(AlterPolicyStmt *stmt) qual = stringToNode(qual_value); /* Add this rel to the parsestate's rangetable, for dependencies */ - addRangeTableEntryForRelation(qual_pstate, target_table, NULL, - false, false); + addRangeTableEntryForRelation(qual_pstate, target_table, + AccessShareLock, + NULL, false, false); qual_parse_rtable = qual_pstate->p_rtable; free_parsestate(qual_pstate); @@ -1137,8 +1145,9 @@ AlterPolicy(AlterPolicyStmt *stmt) with_check_qual = stringToNode(with_check_value); /* Add this rel to the parsestate's rangetable, for dependencies */ - addRangeTableEntryForRelation(with_check_pstate, target_table, NULL, - false, false); + addRangeTableEntryForRelation(with_check_pstate, target_table, + AccessShareLock, + NULL, false, false); with_check_parse_rtable = with_check_pstate->p_rtable; free_parsestate(with_check_pstate); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 29d8353779b..9e60bb37a79 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13632,7 +13632,8 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy) * rangetable entry. We need a ParseState for transformExpr. */ pstate = make_parsestate(NULL); - rte = addRangeTableEntryForRelation(pstate, rel, NULL, false, true); + rte = addRangeTableEntryForRelation(pstate, rel, AccessShareLock, + NULL, false, true); addRTEtoQuery(pstate, rte, true, true, true); /* take care of any partition expressions */ diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 36778b6f4d5..b2de1cc3915 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -577,10 +577,12 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, * 'OLD' must always have varno equal to 1 and 'NEW' equal to 2. */ rte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, makeAlias("old", NIL), false, false); addRTEtoQuery(pstate, rte, false, true, true); rte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, makeAlias("new", NIL), false, false); addRTEtoQuery(pstate, rte, false, true, true); diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index ffb71c0ea7c..e73f1d8894c 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -353,7 +353,7 @@ DefineViewRules(Oid viewOid, Query *viewParse, bool replace) * by 2... * * These extra RT entries are not actually used in the query, - * except for run-time permission checking. + * except for run-time locking and permission checking. *--------------------------------------------------------------- */ static Query * @@ -386,9 +386,11 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse) * OLD first, then NEW.... */ rt_entry1 = addRangeTableEntryForRelation(pstate, viewRel, + AccessShareLock, makeAlias("old", NIL), false, false); rt_entry2 = addRangeTableEntryForRelation(pstate, viewRel, + AccessShareLock, makeAlias("new", NIL), false, false); /* Must override addRangeTableEntry's default access-check flags */ diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 5443cbf67db..2b7cf263803 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -864,6 +864,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) Relation resultRelation; resultRelationOid = getrelid(resultRelationIndex, rangeTable); + Assert(rt_fetch(resultRelationIndex, rangeTable)->rellockmode == RowExclusiveLock); resultRelation = heap_open(resultRelationOid, RowExclusiveLock); InitResultRelInfo(resultRelInfo, @@ -904,6 +905,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) Relation resultRelDesc; resultRelOid = getrelid(resultRelIndex, rangeTable); + Assert(rt_fetch(resultRelIndex, rangeTable)->rellockmode == RowExclusiveLock); resultRelDesc = heap_open(resultRelOid, RowExclusiveLock); InitResultRelInfo(resultRelInfo, resultRelDesc, @@ -924,8 +926,11 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* We locked the roots above. */ if (!list_member_int(plannedstmt->rootResultRelations, resultRelIndex)) + { + Assert(rt_fetch(resultRelIndex, rangeTable)->rellockmode == RowExclusiveLock); LockRelationOid(getrelid(resultRelIndex, rangeTable), RowExclusiveLock); + } } } } @@ -955,6 +960,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) { PlanRowMark *rc = (PlanRowMark *) lfirst(l); Oid relid; + LOCKMODE rellockmode; Relation relation; ExecRowMark *erm; @@ -964,6 +970,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* get relation's OID (will produce InvalidOid if subquery) */ relid = getrelid(rc->rti, rangeTable); + rellockmode = rt_fetch(rc->rti, rangeTable)->rellockmode; /* * If you change the conditions under which rel locks are acquired @@ -975,9 +982,13 @@ InitPlan(QueryDesc *queryDesc, int eflags) case ROW_MARK_NOKEYEXCLUSIVE: case ROW_MARK_SHARE: case ROW_MARK_KEYSHARE: + Assert(rellockmode == RowShareLock); relation = heap_open(relid, RowShareLock); break; case ROW_MARK_REFERENCE: + /* RTE might be a query target table */ + Assert(rellockmode == AccessShareLock || + rellockmode == RowExclusiveLock); relation = heap_open(relid, AccessShareLock); break; case ROW_MARK_COPY: diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 5b3eaec80bc..da84d5df442 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -657,20 +657,32 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags) /* * 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. + * relation. + * + * Note: we may have already gotten the desired lock type, but for now + * don't try to optimize; this logic is going away soon anyhow. */ lockmode = AccessShareLock; if (ExecRelationIsTargetRelation(estate, scanrelid)) - lockmode = NoLock; + lockmode = RowExclusiveLock; else { /* Keep this check in sync with InitPlan! */ ExecRowMark *erm = ExecFindRowMark(estate, scanrelid, true); - if (erm != NULL && erm->relation != NULL) - lockmode = NoLock; + if (erm != NULL) + { + if (erm->markType == ROW_MARK_REFERENCE || + erm->markType == ROW_MARK_COPY) + lockmode = AccessShareLock; + else + lockmode = RowShareLock; + } } + /* lockmode per above logic must not be more than we previously acquired */ + Assert(lockmode <= rt_fetch(scanrelid, estate->es_range_table)->rellockmode); + /* Open the relation and acquire lock as needed */ reloid = getrelid(scanrelid, estate->es_range_table); rel = heap_open(reloid, lockmode); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 7c8220cf651..925cb8f3800 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(rellockmode); 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 378f2facb84..3bb91c95958 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(rellockmode); 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 93f1e2c4ebc..22dbae15d3b 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -3131,6 +3131,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node) case RTE_RELATION: WRITE_OID_FIELD(relid); WRITE_CHAR_FIELD(relkind); + WRITE_INT_FIELD(rellockmode); WRITE_NODE_FIELD(tablesample); break; case RTE_SUBQUERY: diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 519deab63ab..ce556580a5f 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1361,6 +1361,7 @@ _readRangeTblEntry(void) case RTE_RELATION: READ_OID_FIELD(relid); READ_CHAR_FIELD(relkind); + READ_INT_FIELD(rellockmode); READ_NODE_FIELD(tablesample); break; case RTE_SUBQUERY: diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 22c010c19e0..89625f4f5b1 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -6021,6 +6021,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid) rte->rtekind = RTE_RELATION; rte->relid = tableOid; rte->relkind = RELKIND_RELATION; /* Don't be too picky. */ + rte->rellockmode = AccessShareLock; rte->lateral = false; rte->inh = false; rte->inFromCl = true; @@ -6143,6 +6144,7 @@ plan_create_index_workers(Oid tableOid, Oid indexOid) rte->rtekind = RTE_RELATION; rte->relid = tableOid; rte->relkind = RELKIND_RELATION; /* Don't be too picky. */ + rte->rellockmode = AccessShareLock; rte->lateral = false; rte->inh = true; rte->inFromCl = true; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index c0206009559..226927b7ab5 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1037,6 +1037,7 @@ transformOnConflictClause(ParseState *pstate, */ exclRte = addRangeTableEntryForRelation(pstate, targetrel, + RowExclusiveLock, makeAlias("excluded", NIL), false, false); exclRte->relkind = RELKIND_COMPOSITE_TYPE; diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index d6b93f55dfd..660011a3ec3 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -217,6 +217,7 @@ setTargetTable(ParseState *pstate, RangeVar *relation, * Now build an RTE. */ rte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation, + RowExclusiveLock, relation->alias, inh, false); pstate->p_target_rangetblentry = rte; diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 60b8de0f95d..da600dc0ff3 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1208,15 +1208,22 @@ addRangeTableEntry(ParseState *pstate, rte->alias = alias; /* + * Identify the type of lock we'll need on this relation. It's not the + * query's target table (that case is handled elsewhere), so we need + * either RowShareLock if it's locked by FOR UPDATE/SHARE, or plain + * AccessShareLock otherwise. + */ + lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock; + + /* * Get the rel's OID. This access also ensures that we have an up-to-date * relcache entry for the rel. Since this is typically the first access - * to a rel in a statement, be careful to get the right access level - * depending on whether we're doing SELECT FOR UPDATE/SHARE. + * to a rel in a statement, we must open the rel with the proper lockmode. */ - lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock; rel = parserOpenTable(pstate, relation, lockmode); rte->relid = RelationGetRelid(rel); rte->relkind = rel->rd_rel->relkind; + rte->rellockmode = lockmode; /* * Build the list of effective column names using user-supplied aliases @@ -1262,10 +1269,20 @@ addRangeTableEntry(ParseState *pstate, * * This is just like addRangeTableEntry() except that it makes an RTE * given an already-open relation instead of a RangeVar reference. + * + * lockmode is the lock type required for query execution; it must be one + * of AccessShareLock, RowShareLock, or RowExclusiveLock depending on the + * RTE's role within the query. The caller should always hold that lock mode + * or a stronger one. + * + * Note: properly, lockmode should be declared LOCKMODE not int, but that + * would require importing storage/lock.h into parse_relation.h. Since + * LOCKMODE is typedef'd as int anyway, that seems like overkill. */ RangeTblEntry * addRangeTableEntryForRelation(ParseState *pstate, Relation rel, + int lockmode, Alias *alias, bool inh, bool inFromCl) @@ -1275,10 +1292,15 @@ addRangeTableEntryForRelation(ParseState *pstate, Assert(pstate != NULL); + Assert(lockmode == AccessShareLock || + lockmode == RowShareLock || + lockmode == RowExclusiveLock); + rte->rtekind = RTE_RELATION; rte->alias = alias; rte->relid = RelationGetRelid(rel); rte->relkind = rel->rd_rel->relkind; + rte->rellockmode = 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 f5c1e2a0d73..42bf9bfec62 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -2526,7 +2526,9 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString) * relation, but we still need to open it. */ rel = relation_open(relid, NoLock); - rte = addRangeTableEntryForRelation(pstate, rel, NULL, false, true); + rte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, + NULL, false, true); /* no to join list, yes to namespaces */ addRTEtoQuery(pstate, rte, false, true, true); @@ -2635,9 +2637,11 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, * qualification. */ oldrte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, makeAlias("old", NIL), false, false); newrte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, makeAlias("new", NIL), false, false); /* Must override addRangeTableEntry's default access-check flags */ @@ -2733,9 +2737,11 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, * them in the joinlist. */ oldrte = addRangeTableEntryForRelation(sub_pstate, rel, + AccessShareLock, makeAlias("old", NIL), false, false); newrte = addRangeTableEntryForRelation(sub_pstate, rel, + AccessShareLock, makeAlias("new", NIL), false, false); oldrte->requiredPerms = 0; @@ -2938,6 +2944,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, pstate->p_sourcetext = queryString; rte = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, NULL, false, true); diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index acc6498567d..6e420d893cf 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -795,7 +795,8 @@ copy_table(Relation rel) copybuf = makeStringInfo(); pstate = make_parsestate(NULL); - addRangeTableEntryForRelation(pstate, rel, NULL, false, false); + addRangeTableEntryForRelation(pstate, rel, AccessShareLock, + NULL, false, false); attnamelist = make_copy_attnamelist(relmapentry); cstate = BeginCopyFrom(pstate, rel, NULL, false, copy_read_data, attnamelist, NIL); diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 42345f94d33..e247539d7b1 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -199,6 +199,7 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel) rte->rtekind = RTE_RELATION; rte->relid = RelationGetRelid(rel->localrel); rte->relkind = rel->localrel->rd_rel->relkind; + rte->rellockmode = AccessShareLock; estate->es_range_table = list_make1(rte); resultRelInfo = makeNode(ResultRelInfo); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 327e5c33d7a..42cec2aeae8 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -89,6 +89,10 @@ static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist); * These locks will ensure that the relation schemas don't change under us * while we are rewriting and planning the query. * + * Caution: this may modify the querytree, therefore caller should usually + * have done a copyObject() to make a writable copy of the querytree in the + * current memory context. + * * forExecute indicates that the query is about to be executed. * If so, we'll acquire RowExclusiveLock on the query's resultRelation, * RowShareLock on any relation accessed FOR [KEY] UPDATE/SHARE, and @@ -101,13 +105,11 @@ static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist); * forUpdatePushedDown indicates that a pushed-down FOR [KEY] UPDATE/SHARE * applies to the current subquery, requiring all rels to be opened with at * least RowShareLock. This should always be false at the top of the - * recursion. This flag is ignored if forExecute is false. + * recursion. When it is true, we adjust RTE rellockmode fields to reflect + * the higher lock level. This flag is ignored if forExecute is false. * * A secondary purpose of this routine is to fix up JOIN RTE references to - * dropped columns (see details below). Because the RTEs are modified in - * place, it is generally appropriate for the caller of this routine to have - * first done a copyObject() to make a writable copy of the querytree in the - * current memory context. + * dropped columns (see details below). Such RTEs are modified in-place. * * This processing can, and for efficiency's sake should, be skipped when the * querytree has just been built by the parser: parse analysis already got @@ -170,12 +172,22 @@ AcquireRewriteLocks(Query *parsetree, lockmode = AccessShareLock; else if (rt_index == parsetree->resultRelation) lockmode = RowExclusiveLock; - else if (forUpdatePushedDown || - get_parse_rowmark(parsetree, rt_index) != NULL) + else if (forUpdatePushedDown) + { + lockmode = RowShareLock; + /* Upgrade RTE's lock mode to reflect pushed-down lock */ + if (rte->rellockmode == AccessShareLock) + rte->rellockmode = RowShareLock; + } + else if (get_parse_rowmark(parsetree, rt_index) != NULL) lockmode = RowShareLock; else lockmode = AccessShareLock; + Assert(!forExecute || lockmode == rte->rellockmode || + (lockmode == AccessShareLock && + rte->rellockmode == RowExclusiveLock)); + rel = heap_open(rte->relid, lockmode); /* @@ -1593,6 +1605,7 @@ ApplyRetrieveRule(Query *parsetree, /* Clear fields that should not be set in a subquery RTE */ rte->relid = InvalidOid; rte->relkind = 0; + rte->rellockmode = 0; rte->tablesample = NULL; rte->inh = false; /* must not be set for a subquery */ @@ -2920,8 +2933,14 @@ rewriteTargetView(Query *parsetree, Relation view) * very much like what the planner would do to "pull up" the view into the * outer query. Perhaps someday we should refactor things enough so that * we can share code with the planner.) + * + * Be sure to set rellockmode to the correct thing for the target table. + * Since we copied the whole viewquery above, we can just scribble on + * base_rte instead of copying it. */ - new_rte = (RangeTblEntry *) base_rte; + new_rte = base_rte; + new_rte->rellockmode = RowExclusiveLock; + parsetree->rtable = lappend(parsetree->rtable, new_rte); new_rt_index = list_length(parsetree->rtable); @@ -3101,8 +3120,8 @@ rewriteTargetView(Query *parsetree, Relation view) new_exclRte = addRangeTableEntryForRelation(make_parsestate(NULL), base_rel, - makeAlias("excluded", - NIL), + RowExclusiveLock, + makeAlias("excluded", NIL), false, false); new_exclRte->relkind = RELKIND_COMPOSITE_TYPE; new_exclRte->requiredPerms = 0; diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index fc034ce6019..049b20449a1 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -1878,12 +1878,14 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) pkrte->rtekind = RTE_RELATION; pkrte->relid = RelationGetRelid(pk_rel); pkrte->relkind = pk_rel->rd_rel->relkind; + pkrte->rellockmode = AccessShareLock; pkrte->requiredPerms = ACL_SELECT; fkrte = makeNode(RangeTblEntry); fkrte->rtekind = RTE_RELATION; fkrte->relid = RelationGetRelid(fk_rel); fkrte->relkind = fk_rel->rd_rel->relkind; + fkrte->rellockmode = AccessShareLock; fkrte->requiredPerms = ACL_SELECT; for (i = 0; i < riinfo->nkeys; i++) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index eecd64e4b57..f6693eaa796 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1000,6 +1000,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) oldrte->rtekind = RTE_RELATION; oldrte->relid = trigrec->tgrelid; oldrte->relkind = relkind; + oldrte->rellockmode = AccessShareLock; oldrte->alias = makeAlias("old", NIL); oldrte->eref = oldrte->alias; oldrte->lateral = false; @@ -1010,6 +1011,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) newrte->rtekind = RTE_RELATION; newrte->relid = trigrec->tgrelid; newrte->relkind = relkind; + newrte->rellockmode = AccessShareLock; newrte->alias = makeAlias("new", NIL); newrte->eref = newrte->alias; newrte->lateral = false; @@ -3206,6 +3208,7 @@ deparse_context_for(const char *aliasname, Oid relid) rte->rtekind = RTE_RELATION; rte->relid = relid; rte->relkind = RELKIND_RELATION; /* no need for exactness here */ + rte->rellockmode = AccessShareLock; rte->alias = makeAlias(aliasname, NIL); rte->eref = rte->alias; rte->lateral = false; diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 7271b5880b8..f3d06cd0743 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -107,7 +107,7 @@ static void PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue); static void PlanCacheSysCallback(Datum arg, int cacheid, uint32 hashvalue); /* GUC parameter */ -int plan_cache_mode; +int plan_cache_mode; /* * InitPlanCache: initialize module during InitPostgres. @@ -1539,6 +1539,8 @@ AcquireExecutorLocks(List *stmt_list, bool acquire) else lockmode = AccessShareLock; + Assert(lockmode == rte->rellockmode); + if (acquire) LockRelationOid(rte->relid, lockmode); else @@ -1609,6 +1611,8 @@ ScanQueryForLocks(Query *parsetree, bool acquire) lockmode = RowShareLock; else lockmode = AccessShareLock; + Assert(lockmode == rte->rellockmode || + (lockmode == AccessShareLock && rte->rellockmode == RowExclusiveLock)); if (acquire) LockRelationOid(rte->relid, lockmode); else diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 07e0576db74..02a9866f321 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201809241 +#define CATALOG_VERSION_NO 201809301 #endif diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 62209a8f102..200df8e6595 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -973,9 +973,21 @@ typedef struct RangeTblEntry * that the tuple format of the tuplestore is the same as the referenced * relation. This allows plans referencing AFTER trigger transition * tables to be invalidated if the underlying table is altered. + * + * rellockmode is really LOCKMODE, but it's declared int to avoid having + * to include lock-related headers here. It must be RowExclusiveLock if + * the RTE is an INSERT/UPDATE/DELETE target, else RowShareLock if the RTE + * is a SELECT FOR UPDATE/FOR SHARE target, else AccessShareLock. + * + * Note: in some cases, rule expansion may result in RTEs that are marked + * with RowExclusiveLock even though they are not the target of the + * current query; this happens if a DO ALSO rule simply scans the original + * target table. We leave such RTEs with their original lockmode so as to + * avoid getting an additional, lesser lock. */ Oid relid; /* OID of the relation */ char relkind; /* relation kind (see pg_class.relkind) */ + int rellockmode; /* lock level that query requires on the rel */ struct TableSampleClause *tablesample; /* sampling info, or NULL */ /* diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index b9792acdaea..687a7d7b1b9 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -69,6 +69,7 @@ extern RangeTblEntry *addRangeTableEntry(ParseState *pstate, bool inFromCl); extern RangeTblEntry *addRangeTableEntryForRelation(ParseState *pstate, Relation rel, + int lockmode, Alias *alias, bool inh, bool inFromCl); diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c index cab67a60aa9..d492697e88d 100644 --- a/src/test/modules/test_rls_hooks/test_rls_hooks.c +++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c @@ -80,8 +80,8 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation) qual_pstate = make_parsestate(NULL); - rte = addRangeTableEntryForRelation(qual_pstate, relation, NULL, false, - false); + rte = addRangeTableEntryForRelation(qual_pstate, relation, AccessShareLock, + NULL, false, false); addRTEtoQuery(qual_pstate, rte, false, true, true); role = ObjectIdGetDatum(ACL_ID_PUBLIC); @@ -143,8 +143,8 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation) qual_pstate = make_parsestate(NULL); - rte = addRangeTableEntryForRelation(qual_pstate, relation, NULL, false, - false); + rte = addRangeTableEntryForRelation(qual_pstate, relation, AccessShareLock, + NULL, false, false); addRTEtoQuery(qual_pstate, rte, false, true, true); role = ObjectIdGetDatum(ACL_ID_PUBLIC); |