I wrote:
> I started poking at this. I thought that it would be a good cross-check
> to apply just the "front half" of 0001 (i.e., creation and population of
> the RTE lockmode field), and then to insert checks in each of the
> "back half" places (executor, plancache, etc) that the lockmodes they
> are computing today match what is in the RTE.
> Those checks fell over many times in the regression tests.
Here's a version that doesn't fall over (for me), incorporating fixes
as I suggested for points 1 and 2, and weakening the back-half assertions
enough to let points 3 and 4 succeed. I'm inclined to commit this to see
if the buildfarm can find any problems I missed.
Some cosmetic changes:
* I renamed the RTE field to rellockmode in the name of greppability.
* I rearranged the order of the arguments for
addRangeTableEntryForRelation to make it more consistent with the
other addRangeTableEntryXXX functions.
regards, tom lane
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 4f1d365..7dfa327 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 9176f62..4ad5868 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 d06662b..2d5bc8a 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 3d82edb..d5cb62d 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 cee0ef9..2fd17b2 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 29d8353..9e60bb3 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 36778b6..b2de1cc 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 ffb71c0..e73f1d8 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 5443cbf..2b7cf26 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 5b3eaec..da84d5d 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 7c8220c..925cb8f 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 378f2fa..3bb91c9 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 93f1e2c..22dbae1 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 519deab..ce55658 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 22c010c..89625f4 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 c020600..226927b 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 d6b93f5..660011a 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 60b8de0..da600dc 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 f5c1e2a..42bf9bf 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 acc6498..6e420d8 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 42345f9..e247539 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 327e5c3..50788fe 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. This flag is ignored if forExecute is false. If it is true,
+ * though, we adjust RTE rellockmode fields to reflect the higher lock level.
*
* 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 fc034ce..049b204 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 eecd64e..f6693ea 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 7271b58..15df189 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -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 07e0576..1e064d4 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 201809291
#endif
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 62209a8..3056819 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 source, 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 b9792ac..687a7d7 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 cab67a6..d492697 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);
