]> granicus.if.org Git - postgresql/commitdiff
Change rewriter/planner/executor/plancache to depend on RTE rellockmode.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 2 Oct 2018 18:43:01 +0000 (14:43 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 2 Oct 2018 18:43:09 +0000 (14:43 -0400)
Instead of recomputing the required lock levels in all these places,
just use what commit fdba460a2 made the parser store in the RTE fields.
This already simplifies the code measurably in these places, and
follow-on changes will remove a bunch of no-longer-needed infrastructure.

In a few cases, this change causes us to acquire a higher lock level
than we did before.  This is OK primarily because said higher lock level
should've been acquired already at query parse time; thus, we're saving
a useless extra trip through the shared lock manager to acquire a lesser
lock alongside the original lock.  The only known exception to this is
that re-execution of a previously planned SELECT FOR UPDATE/SHARE query,
for a table that uses ROW_MARK_REFERENCE or ROW_MARK_COPY methods, might
have gotten only AccessShareLock before.  Now it will get RowShareLock
like the first execution did, which seems fine.

While there's more to do, push it in this state anyway, to let the
buildfarm help verify that nothing bad happened.

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

src/backend/executor/execMain.c
src/backend/executor/execUtils.c
src/backend/optimizer/prep/prepunion.c
src/backend/rewrite/rewriteHandler.c
src/backend/utils/cache/plancache.c

index c28750e07533d38bb38151db1cdffecf022190c2..c865032a80f1e64af6436ed2a3683f24dc2e263d 100644 (file)
@@ -970,26 +970,16 @@ 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
-                * here, be sure to adjust ExecOpenScanRelation to match.
-                */
                switch (rc->markType)
                {
                        case ROW_MARK_EXCLUSIVE:
                        case ROW_MARK_NOKEYEXCLUSIVE:
                        case ROW_MARK_SHARE:
                        case ROW_MARK_KEYSHARE:
-                               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);
+                               rellockmode = rt_fetch(rc->rti, rangeTable)->rellockmode;
+                               relation = heap_open(relid, rellockmode);
                                break;
                        case ROW_MARK_COPY:
                                /* no physical table access is required */
index da84d5df442fa6fa67a8d7214393de45222a2188..a4058c0f61dd9afb3d24a554666e6b1030d765de 100644 (file)
@@ -641,10 +641,6 @@ ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
  *
  *             Open the heap relation to be scanned by a base-level scan plan node.
  *             This should be called during the node's ExecInit routine.
- *
- * By default, this acquires AccessShareLock on the relation.  However,
- * if the relation was already locked by InitPlan, we don't need to acquire
- * any additional lock.  This saves trips to the shared lock manager.
  * ----------------------------------------------------------------
  */
 Relation
@@ -654,37 +650,9 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
        Oid                     reloid;
        LOCKMODE        lockmode;
 
-       /*
-        * Determine the lock type we need.  First, scan to see if target relation
-        * is a result relation.  If not, check if it's a FOR UPDATE/FOR SHARE
-        * relation.
-        *
-        * 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 = RowExclusiveLock;
-       else
-       {
-               /* Keep this check in sync with InitPlan! */
-               ExecRowMark *erm = ExecFindRowMark(estate, scanrelid, true);
-
-               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);
+       lockmode = rt_fetch(scanrelid, estate->es_range_table)->rellockmode;
        rel = heap_open(reloid, lockmode);
 
        /*
@@ -912,6 +880,7 @@ ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate)
                if (!is_result_rel)
                {
                        PlanRowMark *rc = NULL;
+                       LOCKMODE        lockmode;
 
                        foreach(l, stmt->rowMarks)
                        {
@@ -923,9 +892,13 @@ ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate)
                        }
 
                        if (rc && RowMarkRequiresRowShareLock(rc->markType))
-                               LockRelationOid(relid, RowShareLock);
+                               lockmode = RowShareLock;
                        else
-                               LockRelationOid(relid, AccessShareLock);
+                               lockmode = AccessShareLock;
+
+                       Assert(lockmode == rt_fetch(rti, estate->es_range_table)->rellockmode);
+
+                       LockRelationOid(relid, lockmode);
                }
        }
 }
index 690b6bbab7fcf902a526a43759163af5681f4c3c..d5720518a8186038bc6a9d23c0043c8ef70999d5 100644 (file)
@@ -1515,7 +1515,6 @@ expand_inherited_tables(PlannerInfo *root)
 static void
 expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
 {
-       Query      *parse = root->parse;
        Oid                     parentOID;
        PlanRowMark *oldrc;
        Relation        oldrelation;
@@ -1546,21 +1545,9 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
         * relation named in the query.  However, for each child relation we add
         * to the query, we must obtain an appropriate lock, because this will be
         * the first use of those relations in the parse/rewrite/plan pipeline.
-        *
-        * If the parent relation is the query's result relation, then we need
-        * RowExclusiveLock.  Otherwise, if it's accessed FOR UPDATE/SHARE, we
-        * need RowShareLock; otherwise AccessShareLock.  We can't just grab
-        * AccessShareLock because then the executor would be trying to upgrade
-        * the lock, leading to possible deadlocks.  (This code should match the
-        * parser and rewriter.)
+        * Child rels should use the same lockmode as their parent.
         */
-       oldrc = get_plan_rowmark(root->rowMarks, rti);
-       if (rti == parse->resultRelation)
-               lockmode = RowExclusiveLock;
-       else if (oldrc && RowMarkRequiresRowShareLock(oldrc->markType))
-               lockmode = RowShareLock;
-       else
-               lockmode = AccessShareLock;
+       lockmode = rte->rellockmode;
 
        /* Scan for all members of inheritance set, acquire needed locks */
        inhOIDs = find_all_inheritors(parentOID, lockmode, NULL);
@@ -1582,6 +1569,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
         * PlanRowMark as isParent = true, and generate a new PlanRowMark for each
         * child.
         */
+       oldrc = get_plan_rowmark(root->rowMarks, rti);
        if (oldrc)
                oldrc->isParent = true;
 
index 42cec2aeae873fa2d997a204cec4de651f945e41..43815d26ff57fae4dde90fd25c89b3ce8025325b 100644 (file)
@@ -87,17 +87,14 @@ static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
  * AcquireRewriteLocks -
  *       Acquire suitable locks on all the relations mentioned in the Query.
  *       These locks will ensure that the relation schemas don't change under us
- *       while we are rewriting and planning the query.
+ *       while we are rewriting, planning, and executing 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
- * AccessShareLock on all other relations mentioned.
- *
+ * forExecute indicates that the query is about to be executed.  If so,
+ * we'll acquire the lock modes specified in the RTE rellockmode fields.
  * If forExecute is false, AccessShareLock is acquired on all relations.
  * This case is suitable for ruleutils.c, for example, where we only need
  * schema stability and we don't intend to actually modify any relations.
@@ -162,31 +159,24 @@ AcquireRewriteLocks(Query *parsetree,
 
                                /*
                                 * Grab the appropriate lock type for the relation, and do not
-                                * release it until end of transaction. This protects the
-                                * rewriter and planner against schema changes mid-query.
+                                * release it until end of transaction.  This protects the
+                                * rewriter, planner, and executor against schema changes
+                                * mid-query.
                                 *
-                                * Assuming forExecute is true, this logic must match what the
-                                * executor will do, else we risk lock-upgrade deadlocks.
+                                * If forExecute is false, ignore rellockmode and just use
+                                * AccessShareLock.
                                 */
                                if (!forExecute)
                                        lockmode = AccessShareLock;
-                               else if (rt_index == parsetree->resultRelation)
-                                       lockmode = RowExclusiveLock;
                                else if (forUpdatePushedDown)
                                {
-                                       lockmode = RowShareLock;
                                        /* Upgrade RTE's lock mode to reflect pushed-down lock */
                                        if (rte->rellockmode == AccessShareLock)
                                                rte->rellockmode = RowShareLock;
+                                       lockmode = rte->rellockmode;
                                }
-                               else if (get_parse_rowmark(parsetree, rt_index) != NULL)
-                                       lockmode = RowShareLock;
                                else
-                                       lockmode = AccessShareLock;
-
-                               Assert(!forExecute || lockmode == rte->rellockmode ||
-                                          (lockmode == AccessShareLock &&
-                                               rte->rellockmode == RowExclusiveLock));
+                                       lockmode = rte->rellockmode;
 
                                rel = heap_open(rte->relid, lockmode);
 
index f3d06cd07433e2a10b2681b8910cd5d72d2ddc17..fc7e8dbe26957ea612d0f2ea062350ebb600bc7a 100644 (file)
@@ -1493,7 +1493,6 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
        foreach(lc1, stmt_list)
        {
                PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc1);
-               int                     rt_index;
                ListCell   *lc2;
 
                if (plannedstmt->commandType == CMD_UTILITY)
@@ -1512,14 +1511,9 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
                        continue;
                }
 
-               rt_index = 0;
                foreach(lc2, plannedstmt->rtable)
                {
                        RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc2);
-                       LOCKMODE        lockmode;
-                       PlanRowMark *rc;
-
-                       rt_index++;
 
                        if (rte->rtekind != RTE_RELATION)
                                continue;
@@ -1530,21 +1524,10 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
                         * fail if it's been dropped entirely --- we'll just transiently
                         * acquire a non-conflicting lock.
                         */
-                       if (list_member_int(plannedstmt->resultRelations, rt_index) ||
-                               list_member_int(plannedstmt->nonleafResultRelations, rt_index))
-                               lockmode = RowExclusiveLock;
-                       else if ((rc = get_plan_rowmark(plannedstmt->rowMarks, rt_index)) != NULL &&
-                                        RowMarkRequiresRowShareLock(rc->markType))
-                               lockmode = RowShareLock;
-                       else
-                               lockmode = AccessShareLock;
-
-                       Assert(lockmode == rte->rellockmode);
-
                        if (acquire)
-                               LockRelationOid(rte->relid, lockmode);
+                               LockRelationOid(rte->relid, rte->rellockmode);
                        else
-                               UnlockRelationOid(rte->relid, lockmode);
+                               UnlockRelationOid(rte->relid, rte->rellockmode);
                }
        }
 }
@@ -1586,7 +1569,6 @@ static void
 ScanQueryForLocks(Query *parsetree, bool acquire)
 {
        ListCell   *lc;
-       int                     rt_index;
 
        /* Shouldn't get called on utility commands */
        Assert(parsetree->commandType != CMD_UTILITY);
@@ -1594,29 +1576,18 @@ ScanQueryForLocks(Query *parsetree, bool acquire)
        /*
         * First, process RTEs of the current query level.
         */
-       rt_index = 0;
        foreach(lc, parsetree->rtable)
        {
                RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
-               LOCKMODE        lockmode;
 
-               rt_index++;
                switch (rte->rtekind)
                {
                        case RTE_RELATION:
                                /* Acquire or release the appropriate type of lock */
-                               if (rt_index == parsetree->resultRelation)
-                                       lockmode = RowExclusiveLock;
-                               else if (get_parse_rowmark(parsetree, rt_index) != NULL)
-                                       lockmode = RowShareLock;
-                               else
-                                       lockmode = AccessShareLock;
-                               Assert(lockmode == rte->rellockmode ||
-                                          (lockmode == AccessShareLock && rte->rellockmode == RowExclusiveLock));
                                if (acquire)
-                                       LockRelationOid(rte->relid, lockmode);
+                                       LockRelationOid(rte->relid, rte->rellockmode);
                                else
-                                       UnlockRelationOid(rte->relid, lockmode);
+                                       UnlockRelationOid(rte->relid, rte->rellockmode);
                                break;
 
                        case RTE_SUBQUERY: