Re: executor relation handling

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: executor relation handling
Дата
Msg-id 23870.1538258741@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: executor relation handling  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
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);

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Arthur Zakirov
Дата:
Сообщение: Re: libpq host/hostaddr/conninfo inconsistencies
Следующее
От: Pavel Stehule
Дата:
Сообщение: Re: [HACKERS] proposal: schema variables