Create an RTE field to record the query's lock mode for each relation.

Add RangeTblEntry.rellockmode, which records the appropriate lock mode for
each RTE_RELATION rangetable entry (either AccessShareLock, RowShareLock,
or RowExclusiveLock depending on the RTE's role in the query).

This patch creates the field and makes all creators of RTE nodes fill it
in reasonably, but for the moment nothing much is done with it.  The plan
is to replace assorted post-parser logic that re-determines the right
lockmode to use with simple uses of rte->rellockmode.  For now, just add
Asserts in each of those places that the rellockmode matches what they are
computing today.  (In some cases the match isn't perfect, so the Asserts
are weaker than you might expect; but this seems OK, as per discussion.)

This passes check-world for me, but it seems worth pushing in this state
to see if the buildfarm finds any problems in cases I failed to test.

catversion bump due to change of stored rules.

Amit Langote, reviewed by David Rowley and Jesper Pedersen,
and whacked around a bit more by me

Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 4f1d3653575b4fc9e6c849b39f0e360f69445dbe..7dfa3278a5b302c38d939e862f9a6d1e637f23a6 100644 (file)
--- 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 9176f6280b0461bc5301d394077ed333a32cdbaa..4ad58689fc32e82e8ad941f35303c247f227ec4d 100644 (file)
--- 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 d06662bd32eb86d4a4ac8880b78f1d777b843695..2d5bc8add68c8b63d665264e9c82ac6a90f18588 100644 (file)
--- 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 3d82edbf581f75b3a5800e0337f54edca39c1006..d5cb62da15bd1945c69982203f6d5064b73de903 100644 (file)
--- 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 cee0ef915be955c97869a40211958fe53c2061d3..2fd17b24b9efbfd0ec56611afc83f540ea73709d 100644 (file)
--- 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 29d8353779b53cd0bd669b30b8876d50f4cd6cd0..9e60bb37a7948a12ff183e039e87da98e5f96a4a 100644 (file)
--- 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 36778b6f4d54e3db36cfc67f0cd461063c5b6376..b2de1cc391502461d99b6865928b96894f291029 100644 (file)
--- 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 ffb71c0ea7c292b474c8c16035289ca5d59b056d..e73f1d8894cf745905c3215f8a25eadab20b85da 100644 (file)
--- 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 5443cbf67db356ad7af6e2acb5aedd6be34cb59f..2b7cf2638039e2ba5d4207b204daf31bd4abd535 100644 (file)
--- 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 5b3eaec80bcafaeff5567c2c7376e9cfe41ecfe6..da84d5df442fa6fa67a8d7214393de45222a2188 100644 (file)
--- 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 7c8220cf6511ec7f3b09b854d0ecbffff1c39c3f..925cb8f3800a7dcc357c11df69c6872da00d1e0c 100644 (file)
--- 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 378f2facb84329d5f55297cf6ec04970467f3673..3bb91c95958e734be4856d3a1445dff0e6af635a 100644 (file)
--- 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 93f1e2c4ebcbe8a464c2114ffad9038df7161b11..22dbae15d3bd2ce01bfde7bd8497076b09c02081 100644 (file)
--- 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 519deab63ab23dba2f9cd82db1b44c7b39a90cf2..ce556580a5f06774d6304c646ed38871b048f90d 100644 (file)
--- 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 22c010c19e08612d89f687296c6b5a52f72ba6db..89625f4f5b18dca8a0f3256c7bef603cd32f064e 100644 (file)
--- 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 c020600955910ad29e7f72da11c44d68b6752c7c..226927b7ab54816fecfc2943935380e034ba37b3 100644 (file)
--- 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 d6b93f55dfd51c192206e5f17610a097ec492f65..660011a3ec3992bcf9d535987e8266900800eecc 100644 (file)
--- 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 60b8de0f95d375b87dc5779e12b7834cc6dab6f4..da600dc0ff3eb4ce2d281ff924ea04815f300812 100644 (file)
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1207,16 +1207,23 @@ addRangeTableEntry(ParseState *pstate,
        rte->rtekind = RTE_RELATION;
        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 f5c1e2a0d73ddf6ca723a4970767c1666784d1e2..42bf9bfec628b82f3fd11c7bd1d554d80263f9e2 100644 (file)
--- 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 acc6498567d07c3e93d0adf5039a10c693e38f64..6e420d893cf70965d0cdf84dd923dc1901c7af62 100644 (file)
--- 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 42345f94d33450e85c036e61830d3cf4c637b6be..e247539d7b158d5a05f0c7d886038c8f59624ec1 100644 (file)
--- 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 327e5c33d7ae7c5531fa902b9087bcaeda66bb6d..42cec2aeae873fa2d997a204cec4de651f945e41 100644 (file)
--- 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 fc034ce601929e0ea40b0b1dcad6371665961872..049b20449a141eb6d5e368e9be0ad909522d46f6 100644 (file)
--- 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 eecd64e4b57266d66524d4030cf119f5a898bb4f..f6693eaa796e91291c7575d42fba705301b6416f 100644 (file)
--- 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 7271b5880b812b3702b744ef18a53c1a8e1f3c7a..f3d06cd07433e2a10b2681b8910cd5d72d2ddc17 100644 (file)
--- 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 07e0576db741c8c076acbfe9106b81d51549f84f..02a9866f321c5c7da4bb6ba5d461c2c177b7f628 100644 (file)
--- 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 62209a8f102c7f0559dcc4bf45cee1f9c4f19154..200df8e6595fc0ddebf6c200339c1bc23130662e 100644 (file)
--- 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 b9792acdaea5534cb538d33522a8de243ac362fe..687a7d7b1b9686d5f131fa1eb6020b2d6ee574a8 100644 (file)
--- 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 cab67a60aa982ae9d1a60463cf34c397826e4a65..d492697e88d1ed430f9efa3517e8f9c49d761702 100644 (file)
--- 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);