On 2018/10/04 5:16, Tom Lane wrote: > I wrote: >> Amit Langote <langote_amit...@lab.ntt.co.jp> writes: >>> Should this check that we're not in a parallel worker process? > >> Hmm. I've not seen any failures in the parallel parts of the regular >> regression tests, but maybe I'd better do a force_parallel_mode >> run before committing. >> In general, I'm not on board with the idea that parallel workers don't >> need to get their own locks, so I don't really want to exclude parallel >> workers from this check. But if it's not safe for that today, fixing it >> is beyond the scope of this particular patch. > > So the place where that came out in the wash is the commit I just made > (9a3cebeaa) to change the executor from taking table locks to asserting > that somebody else took them already.
Thanks for getting that done. > To make that work, I had to make > both ExecOpenScanRelation and relation_open skip checking for lock-held > if IsParallelWorker(). Yeah, I had to do that to when rebasing the remaining patches. > This makes me entirely uncomfortable with the idea that parallel workers > can be allowed to not take any locks of their own. There is no basis > for arguing that we have field proof that that's safe, because *up to > now, parallel workers in fact did take their own locks*. And it seems > unsafe on its face, because there's nothing that really guarantees that > the parent process won't go away while children are still running. > (elog(FATAL) seems like a counterexample, for instance.) > > I think that we ought to adjust parallel query to insist that children > do take locks, and then revert the IsParallelWorker() exceptions I made > here. Maybe I'm missing something here, but isn't the necessary adjustment just that the relations are opened with locks if inside a parallel worker? > I plan to leave that point in abeyance till we've got the rest > of these changes in place, though. The easiest way to do it will > doubtless change once we've centralized the executor's table-opening > logic, so trying to code it right now seems like a waste of effort. Okay. I've rebased the remaining patches. I broke down one of the patches into 2 and re-ordered the patches as follows: 0001: introduces a function that opens range table relations and maintains them in an array indexes by RT index 0002: introduces a new field in EState that's an array of RangeTblEntry pointers and revises macros used in the executor that access RTEs to return them from the array (David Rowley co-authored this one) 0003: moves result relation and ExecRowMark initialization out of InitPlan and into ExecInit* routines of respective nodes 0004: removes useless fields from certain planner nodes whose only purpose has been to assist the executor lock relations in proper order 0005: teaches planner to remove PlanRowMarks corresponding to dummy relations Thanks, Amit
From ebef0d923ea8a1d5e458c9a60845bad68904cb52 Mon Sep 17 00:00:00 2001 From: "dgrow...@gmail.com" <dgrow...@gmail.com> Date: Thu, 27 Sep 2018 16:14:41 +1200 Subject: [PATCH v12 1/5] Revise executor range table relation locking and opening/closing All requests to open range table relations in the executor now go through ExecRangeTableRelation(), which consults an array of Relation pointers indexed by RT index, an arrangement which allows the executor to have to heap_open any given range table relation only once. Relations are closed by ExecEndPlan() instead of ExecEndNode. This needed revising PartitionedRelPruneInfo node to contain the partitioned table's RT index instead of OID. With that change, ExecCreatePartitionPruneState can use ExecRangeTableRelation described above. --- contrib/postgres_fdw/postgres_fdw.c | 4 -- src/backend/executor/README | 4 +- src/backend/executor/execMain.c | 61 +++++++++++++------------------ src/backend/executor/execPartition.c | 34 ++--------------- src/backend/executor/execUtils.c | 60 +++++++++++++++--------------- src/backend/executor/nodeAppend.c | 6 --- src/backend/executor/nodeBitmapHeapscan.c | 7 ---- src/backend/executor/nodeCustom.c | 4 -- src/backend/executor/nodeForeignscan.c | 4 -- src/backend/executor/nodeIndexonlyscan.c | 7 ---- src/backend/executor/nodeIndexscan.c | 7 ---- src/backend/executor/nodeMergeAppend.c | 6 --- src/backend/executor/nodeModifyTable.c | 4 +- src/backend/executor/nodeSamplescan.c | 5 --- src/backend/executor/nodeSeqscan.c | 7 ---- src/backend/executor/nodeTidscan.c | 5 --- src/backend/nodes/copyfuncs.c | 2 +- src/backend/nodes/outfuncs.c | 2 +- src/backend/nodes/readfuncs.c | 2 +- src/backend/optimizer/plan/setrefs.c | 30 +++++++++++++++ src/backend/partitioning/partprune.c | 5 +-- src/include/executor/execPartition.h | 1 - src/include/executor/executor.h | 2 +- src/include/nodes/execnodes.h | 2 + src/include/nodes/plannodes.h | 2 +- 25 files changed, 100 insertions(+), 173 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 6cbba97c22..c02287a55c 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2546,10 +2546,6 @@ postgresEndDirectModify(ForeignScanState *node) ReleaseConnection(dmstate->conn); dmstate->conn = NULL; - /* close the target relation. */ - if (dmstate->resultRel) - ExecCloseScanRelation(dmstate->resultRel); - /* MemoryContext will be deleted automatically. */ } diff --git a/src/backend/executor/README b/src/backend/executor/README index 0d7cd552eb..30df410e0e 100644 --- a/src/backend/executor/README +++ b/src/backend/executor/README @@ -267,8 +267,8 @@ This is a sketch of control flow for full query processing: Per above comments, it's not really critical for ExecEndNode to free any memory; it'll all go away in FreeExecutorState anyway. However, we do need to -be careful to close relations, drop buffer pins, etc, so we do need to scan -the plan state tree to find these sorts of resources. +be careful to drop buffer pins, etc, so we do need to scan the plan state tree +to find these sorts of resources. The executor can also be used to evaluate simple expressions without any Plan diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 9569d2fa42..4ec47ac41e 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -824,6 +824,15 @@ InitPlan(QueryDesc *queryDesc, int eflags) * initialize the node's execution state */ estate->es_range_table = rangeTable; + + /* + * Allocate an array to store open Relations of each rangeTable item. We + * zero-fill this for now. The Relations are only opened and stored here + * the first time they're required during node initialization. + */ + estate->es_relations = (Relation *) palloc0(list_length(rangeTable) * + sizeof(Relation)); + estate->es_plannedstmt = plannedstmt; /* @@ -845,13 +854,10 @@ InitPlan(QueryDesc *queryDesc, int eflags) foreach(l, resultRelations) { Index resultRelationIndex = lfirst_int(l); - Oid resultRelationOid; Relation resultRelation; - resultRelationOid = getrelid(resultRelationIndex, rangeTable); - resultRelation = heap_open(resultRelationOid, NoLock); - Assert(CheckRelationLockedByMe(resultRelation, RowExclusiveLock, true)); - + Assert(rt_fetch(resultRelationIndex, rangeTable)->rellockmode == RowExclusiveLock); + resultRelation = ExecRangeTableRelation(estate, resultRelationIndex); InitResultRelInfo(resultRelInfo, resultRelation, resultRelationIndex, @@ -886,12 +892,10 @@ InitPlan(QueryDesc *queryDesc, int eflags) foreach(l, plannedstmt->rootResultRelations) { Index resultRelIndex = lfirst_int(l); - Oid resultRelOid; Relation resultRelDesc; - resultRelOid = getrelid(resultRelIndex, rangeTable); - resultRelDesc = heap_open(resultRelOid, NoLock); - Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); + Assert(rt_fetch(resultRelIndex, rangeTable)->rellockmode == RowExclusiveLock); + resultRelDesc = ExecRangeTableRelation(estate, resultRelIndex); InitResultRelInfo(resultRelInfo, resultRelDesc, lfirst_int(l), @@ -967,10 +971,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) case ROW_MARK_SHARE: case ROW_MARK_KEYSHARE: case ROW_MARK_REFERENCE: - relation = heap_open(relid, NoLock); - Assert(CheckRelationLockedByMe(relation, - rt_fetch(rc->rti, rangeTable)->rellockmode, - true)); + relation = ExecRangeTableRelation(estate, rc->rti); break; case ROW_MARK_COPY: /* no physical table access is required */ @@ -1606,10 +1607,18 @@ ExecPostprocessPlan(EState *estate) static void ExecEndPlan(PlanState *planstate, EState *estate) { + int num_relations = list_length(estate->es_range_table); ResultRelInfo *resultRelInfo; int i; ListCell *l; + /* Close range table relations. */ + for (i = 0; i < num_relations; i++) + { + if (estate->es_relations[i]) + heap_close(estate->es_relations[i], NoLock); + } + /* * shut down the node-type-specific query processing */ @@ -1634,39 +1643,18 @@ ExecEndPlan(PlanState *planstate, EState *estate) ExecResetTupleTable(estate->es_tupleTable, false); /* - * close the result relation(s) if any, but hold locks until xact commit. + * close indexes of result relation(s) if any */ resultRelInfo = estate->es_result_relations; for (i = estate->es_num_result_relations; i > 0; i--) { - /* Close indices and then the relation itself */ + /* Close indices; the relation itself already closed above */ ExecCloseIndices(resultRelInfo); - heap_close(resultRelInfo->ri_RelationDesc, NoLock); - resultRelInfo++; - } - - /* Close the root target relation(s). */ - resultRelInfo = estate->es_root_result_relations; - for (i = estate->es_num_root_result_relations; i > 0; i--) - { - heap_close(resultRelInfo->ri_RelationDesc, NoLock); resultRelInfo++; } /* likewise close any trigger target relations */ ExecCleanUpTriggerState(estate); - - /* - * close any relations selected FOR [KEY] UPDATE/SHARE, again keeping - * locks - */ - foreach(l, estate->es_rowMarks) - { - ExecRowMark *erm = (ExecRowMark *) lfirst(l); - - if (erm->relation) - heap_close(erm->relation, NoLock); - } } /* ---------------------------------------------------------------- @@ -3161,6 +3149,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) estate->es_snapshot = parentestate->es_snapshot; estate->es_crosscheck_snapshot = parentestate->es_crosscheck_snapshot; estate->es_range_table = parentestate->es_range_table; + estate->es_relations = parentestate->es_relations; estate->es_plannedstmt = parentestate->es_plannedstmt; estate->es_junkFilter = parentestate->es_junkFilter; estate->es_output_cid = parentestate->es_output_cid; diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 832c79b41e..c082bb7632 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -1389,9 +1389,6 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map) * functions. Details stored include how to map the partition index * returned by the partition pruning code into subplan indexes. * - * ExecDestroyPartitionPruneState: - * Deletes a PartitionPruneState. Must be called during executor shutdown. - * * ExecFindInitialMatchingSubPlans: * Returns indexes of matching subplans. Partition pruning is attempted * without any evaluation of expressions containing PARAM_EXEC Params. @@ -1433,6 +1430,7 @@ PartitionPruneState * ExecCreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *partitionpruneinfo) { + EState *estate = planstate->state; PartitionPruneState *prunestate; int n_part_hierarchies; ListCell *lc; @@ -1511,11 +1509,9 @@ ExecCreatePartitionPruneState(PlanState *planstate, /* * We need to hold a pin on the partitioned table's relcache entry * so that we can rely on its copies of the table's partition key - * and partition descriptor. We need not get a lock though; one - * should have been acquired already by InitPlan or - * ExecLockNonLeafAppendTables. + * and partition descriptor. */ - context->partrel = relation_open(pinfo->reloid, NoLock); + context->partrel = ExecRangeTableRelation(estate, pinfo->rtindex); partkey = RelationGetPartitionKey(context->partrel); partdesc = RelationGetPartitionDesc(context->partrel); @@ -1596,30 +1592,6 @@ ExecCreatePartitionPruneState(PlanState *planstate, } /* - * ExecDestroyPartitionPruneState - * Release resources at plan shutdown. - * - * We don't bother to free any memory here, since the whole executor context - * will be going away shortly. We do need to release our relcache pins. - */ -void -ExecDestroyPartitionPruneState(PartitionPruneState *prunestate) -{ - PartitionPruningData **partprunedata = prunestate->partprunedata; - int i; - - for (i = 0; i < prunestate->num_partprunedata; i++) - { - PartitionPruningData *prunedata = partprunedata[i]; - PartitionedRelPruningData *pprune = prunedata->partrelprunedata; - int j; - - for (j = 0; j < prunedata->num_partrelprunedata; j++) - relation_close(pprune[j].context.partrel, NoLock); - } -} - -/* * ExecFindInitialMatchingSubPlans * Identify the set of subplans that cannot be eliminated by initial * pruning (disregarding any pruning constraints involving PARAM_EXEC diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index ba93b40104..4825756a32 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -25,7 +25,6 @@ * etc * * ExecOpenScanRelation Common code for scan node init routines. - * ExecCloseScanRelation * * executor_errposition Report syntactic position of an error. * @@ -648,15 +647,9 @@ Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags) { Relation rel; - Oid reloid; - /* Open the relation and verify lock was obtained upstream */ - reloid = getrelid(scanrelid, estate->es_range_table); - rel = heap_open(reloid, NoLock); - Assert(IsParallelWorker() || - CheckRelationLockedByMe(rel, - rt_fetch(scanrelid, estate->es_range_table)->rellockmode, - true)); + /* Open the relation. */ + rel = ExecRangeTableRelation(estate, scanrelid); /* * Complain if we're attempting a scan of an unscannable relation, except @@ -674,26 +667,6 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags) return rel; } -/* ---------------------------------------------------------------- - * ExecCloseScanRelation - * - * Close the heap relation scanned by a base-level scan plan node. - * This should be called during the node's ExecEnd routine. - * - * Currently, we do not release the lock acquired by ExecOpenScanRelation. - * This lock should be held till end of transaction. (There is a faction - * that considers this too much locking, however.) - * - * If we did want to release the lock, we'd have to repeat the logic in - * ExecOpenScanRelation in order to figure out what to release. - * ---------------------------------------------------------------- - */ -void -ExecCloseScanRelation(Relation scanrel) -{ - heap_close(scanrel, NoLock); -} - /* * UpdateChangedParamSet * Add changed parameters to a plan node's chgParam set @@ -1056,3 +1029,32 @@ ExecCleanTargetListLength(List *targetlist) } return len; } + +/* + * ExecRangeTableRelation + * Open and lock, if not already done, a range table relation + */ +Relation +ExecRangeTableRelation(EState *estate, Index rti) +{ + Relation rel = estate->es_relations[rti - 1]; + + if (rel == NULL) + { + RangeTblEntry *rte = rt_fetch(rti, estate->es_range_table); + + Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock); + + /* Open the relation and verify lock was obtained upstream */ + rel = estate->es_relations[rti - 1] = heap_open(rte->relid, NoLock); + + /* + * In the case of parallel query, only the leader would've obtained + * the lock. + */ + Assert(IsParallelWorker() || + CheckRelationLockedByMe(rel, rte->rellockmode, false)); + } + + return rel; +} diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index f08dfcbcf0..d44befd44e 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -335,12 +335,6 @@ ExecEndAppend(AppendState *node) */ for (i = 0; i < nplans; i++) ExecEndNode(appendplans[i]); - - /* - * release any resources associated with run-time pruning - */ - if (node->as_prune_state) - ExecDestroyPartitionPruneState(node->as_prune_state); } void diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index baffae27e3..5307cd1b87 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -785,13 +785,11 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node) void ExecEndBitmapHeapScan(BitmapHeapScanState *node) { - Relation relation; HeapScanDesc scanDesc; /* * extract information from the node */ - relation = node->ss.ss_currentRelation; scanDesc = node->ss.ss_currentScanDesc; /* @@ -832,11 +830,6 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) * close heap scan */ heap_endscan(scanDesc); - - /* - * close the heap relation. - */ - ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c index b816e0b31d..9a33eda688 100644 --- a/src/backend/executor/nodeCustom.c +++ b/src/backend/executor/nodeCustom.c @@ -126,10 +126,6 @@ ExecEndCustomScan(CustomScanState *node) /* Clean out the tuple table */ ExecClearTuple(node->ss.ps.ps_ResultTupleSlot); ExecClearTuple(node->ss.ss_ScanTupleSlot); - - /* Close the heap relation */ - if (node->ss.ss_currentRelation) - ExecCloseScanRelation(node->ss.ss_currentRelation); } void diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index a2a28b7ec2..cf7df72d8c 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -258,10 +258,6 @@ ExecEndForeignScan(ForeignScanState *node) /* clean out the tuple table */ ExecClearTuple(node->ss.ps.ps_ResultTupleSlot); ExecClearTuple(node->ss.ss_ScanTupleSlot); - - /* close the relation. */ - if (node->ss.ss_currentRelation) - ExecCloseScanRelation(node->ss.ss_currentRelation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c index 4b6d531810..1b530cea40 100644 --- a/src/backend/executor/nodeIndexonlyscan.c +++ b/src/backend/executor/nodeIndexonlyscan.c @@ -373,14 +373,12 @@ ExecEndIndexOnlyScan(IndexOnlyScanState *node) { Relation indexRelationDesc; IndexScanDesc indexScanDesc; - Relation relation; /* * extract information from the node */ indexRelationDesc = node->ioss_RelationDesc; indexScanDesc = node->ioss_ScanDesc; - relation = node->ss.ss_currentRelation; /* Release VM buffer pin, if any. */ if (node->ioss_VMBuffer != InvalidBuffer) @@ -411,11 +409,6 @@ ExecEndIndexOnlyScan(IndexOnlyScanState *node) index_endscan(indexScanDesc); if (indexRelationDesc) index_close(indexRelationDesc, NoLock); - - /* - * close the heap relation. - */ - ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index 6285a2114e..786e7ac25c 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -802,14 +802,12 @@ ExecEndIndexScan(IndexScanState *node) { Relation indexRelationDesc; IndexScanDesc indexScanDesc; - Relation relation; /* * extract information from the node */ indexRelationDesc = node->iss_RelationDesc; indexScanDesc = node->iss_ScanDesc; - relation = node->ss.ss_currentRelation; /* * Free the exprcontext(s) ... now dead code, see ExecFreeExprContext @@ -833,11 +831,6 @@ ExecEndIndexScan(IndexScanState *node) index_endscan(indexScanDesc); if (indexRelationDesc) index_close(indexRelationDesc, NoLock); - - /* - * close the heap relation. - */ - ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c index 9a72d3a0ac..6daf60a454 100644 --- a/src/backend/executor/nodeMergeAppend.c +++ b/src/backend/executor/nodeMergeAppend.c @@ -369,12 +369,6 @@ ExecEndMergeAppend(MergeAppendState *node) */ for (i = 0; i < nplans; i++) ExecEndNode(mergeplans[i]); - - /* - * release any resources associated with run-time pruning - */ - if (node->ms_prune_state) - ExecDestroyPartitionPruneState(node->ms_prune_state); } void diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 24beb40435..07ac3de5e0 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2690,9 +2690,7 @@ ExecEndModifyTable(ModifyTableState *node) { int i; - /* - * Allow any FDWs to shut down - */ + /* Allow any FDWs to shut down */ for (i = 0; i < node->mt_nplans; i++) { ResultRelInfo *resultRelInfo = node->resultRelInfo + i; diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c index 99528be84a..92ff68a764 100644 --- a/src/backend/executor/nodeSamplescan.c +++ b/src/backend/executor/nodeSamplescan.c @@ -222,11 +222,6 @@ ExecEndSampleScan(SampleScanState *node) */ if (node->ss.ss_currentScanDesc) heap_endscan(node->ss.ss_currentScanDesc); - - /* - * close the heap relation. - */ - ExecCloseScanRelation(node->ss.ss_currentRelation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index cd53491be0..92ff1e50ed 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -201,13 +201,11 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags) void ExecEndSeqScan(SeqScanState *node) { - Relation relation; HeapScanDesc scanDesc; /* * get information from node */ - relation = node->ss.ss_currentRelation; scanDesc = node->ss.ss_currentScanDesc; /* @@ -226,11 +224,6 @@ ExecEndSeqScan(SeqScanState *node) */ if (scanDesc != NULL) heap_endscan(scanDesc); - - /* - * close the heap relation. - */ - ExecCloseScanRelation(relation); } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index 0cb1946a3e..8cae56f48c 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -489,11 +489,6 @@ ExecEndTidScan(TidScanState *node) */ ExecClearTuple(node->ss.ps.ps_ResultTupleSlot); ExecClearTuple(node->ss.ss_ScanTupleSlot); - - /* - * close the heap relation. - */ - ExecCloseScanRelation(node->ss.ss_currentRelation); } /* ---------------------------------------------------------------- diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 925cb8f380..3b690b55b3 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1193,7 +1193,7 @@ _copyPartitionedRelPruneInfo(const PartitionedRelPruneInfo *from) { PartitionedRelPruneInfo *newnode = makeNode(PartitionedRelPruneInfo); - COPY_SCALAR_FIELD(reloid); + COPY_SCALAR_FIELD(rtindex); COPY_NODE_FIELD(pruning_steps); COPY_BITMAPSET_FIELD(present_parts); COPY_SCALAR_FIELD(nparts); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 22dbae15d3..863d29cc57 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1032,7 +1032,7 @@ _outPartitionedRelPruneInfo(StringInfo str, const PartitionedRelPruneInfo *node) WRITE_NODE_TYPE("PARTITIONEDRELPRUNEINFO"); - WRITE_OID_FIELD(reloid); + WRITE_UINT_FIELD(rtindex); WRITE_NODE_FIELD(pruning_steps); WRITE_BITMAPSET_FIELD(present_parts); WRITE_INT_FIELD(nparts); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index ce556580a5..73ffa9714c 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -2376,7 +2376,7 @@ _readPartitionedRelPruneInfo(void) { READ_LOCALS(PartitionedRelPruneInfo); - READ_OID_FIELD(reloid); + READ_UINT_FIELD(rtindex); READ_NODE_FIELD(pruning_steps); READ_BITMAPSET_FIELD(present_parts); READ_INT_FIELD(nparts); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index f66f39d8c6..13ee458802 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -925,6 +925,21 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) (Plan *) lfirst(l), rtoffset); } + if (splan->part_prune_info) + { + foreach(l, splan->part_prune_info->prune_infos) + { + List *prune_infos = lfirst(l); + ListCell *l2; + + foreach(l2, prune_infos) + { + PartitionedRelPruneInfo *pinfo = lfirst(l2); + + pinfo->rtindex += rtoffset; + } + } + } } break; case T_MergeAppend: @@ -947,6 +962,21 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) (Plan *) lfirst(l), rtoffset); } + if (splan->part_prune_info) + { + foreach(l, splan->part_prune_info->prune_infos) + { + List *prune_infos = lfirst(l); + ListCell *l2; + + foreach(l2, prune_infos) + { + PartitionedRelPruneInfo *pinfo = lfirst(l2); + + pinfo->rtindex += rtoffset; + } + } + } } break; case T_RecursiveUnion: diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index e1ce8b4ddc..1763dd67a3 100644 --- a/src/backend/partitioning/partprune.c +++ b/src/backend/partitioning/partprune.c @@ -357,7 +357,6 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel, Index rti = lfirst_int(lc); RelOptInfo *subpart = find_base_rel(root, rti); PartitionedRelPruneInfo *pinfo; - RangeTblEntry *rte; Bitmapset *present_parts; int nparts = subpart->nparts; int partnatts = subpart->part_scheme->partnatts; @@ -459,10 +458,8 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel, present_parts = bms_add_member(present_parts, i); } - rte = root->simple_rte_array[subpart->relid]; - pinfo = makeNode(PartitionedRelPruneInfo); - pinfo->reloid = rte->relid; + pinfo->rtindex = rti; pinfo->pruning_steps = pruning_steps; pinfo->present_parts = present_parts; pinfo->nparts = nparts; diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h index 8b4a9ca044..3e08104ea4 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -195,7 +195,6 @@ extern void ExecCleanupTupleRouting(ModifyTableState *mtstate, PartitionTupleRouting *proute); extern PartitionPruneState *ExecCreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *partitionpruneinfo); -extern void ExecDestroyPartitionPruneState(PartitionPruneState *prunestate); extern Bitmapset *ExecFindMatchingSubPlans(PartitionPruneState *prunestate); extern Bitmapset *ExecFindInitialMatchingSubPlans(PartitionPruneState *prunestate, int nsubplans); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index f82b51667f..f3df6408a9 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -513,7 +513,6 @@ extern void ExecCreateScanSlotFromOuterPlan(EState *estate, ScanState *scanstate extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid); extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags); -extern void ExecCloseScanRelation(Relation scanrel); extern int executor_errposition(EState *estate, int location); @@ -569,5 +568,6 @@ extern void CheckCmdReplicaIdentity(Relation rel, CmdType cmd); extern void CheckSubscriptionRelkind(char relkind, const char *nspname, const char *relname); +extern Relation ExecRangeTableRelation(EState *estate, Index rti); #endif /* EXECUTOR_H */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 020ecdcd40..7c47967494 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -479,6 +479,8 @@ typedef struct EState Snapshot es_snapshot; /* time qual to use */ Snapshot es_crosscheck_snapshot; /* crosscheck time qual for RI */ List *es_range_table; /* List of RangeTblEntry */ + Relation *es_relations; /* 0-based array of Relations + * es_range_table_size elements in length. */ PlannedStmt *es_plannedstmt; /* link to top of plan tree */ const char *es_sourceText; /* Source text from QueryDesc */ diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 7c2abbd03a..fbbb278e79 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1105,7 +1105,7 @@ typedef struct PartitionPruneInfo typedef struct PartitionedRelPruneInfo { NodeTag type; - Oid reloid; /* OID of partition rel for this level */ + Index rtindex; /* Partition table's RT index */ List *pruning_steps; /* List of PartitionPruneStep, see below */ Bitmapset *present_parts; /* Indexes of all partitions which subplans or * subparts are present for. */ -- 2.11.0
From c89a0b53040d680eefe4e7178e67a2fe878a24f1 Mon Sep 17 00:00:00 2001 From: amit <amitlangot...@gmail.com> Date: Wed, 3 Oct 2018 16:05:15 +0900 Subject: [PATCH v12 2/5] Introduce an array of RangeTblEntry pointers in EState Author: Amit Langote, David Rowley --- contrib/postgres_fdw/postgres_fdw.c | 12 ++++++------ src/backend/commands/copy.c | 2 +- src/backend/commands/trigger.c | 3 ++- src/backend/executor/execExprInterp.c | 7 ++++--- src/backend/executor/execMain.c | 19 +++++++++++-------- src/backend/executor/execUtils.c | 24 +++++++++++++++++++++++- src/backend/executor/nodeLockRows.c | 2 +- src/backend/replication/logical/worker.c | 2 ++ src/include/executor/executor.h | 1 + src/include/nodes/execnodes.h | 19 +++++++++++++++++++ src/include/parser/parsetree.h | 10 ---------- 11 files changed, 70 insertions(+), 31 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index c02287a55c..3dd1bab2a2 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1345,7 +1345,7 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags) rtindex = fsplan->scan.scanrelid; else rtindex = bms_next_member(fsplan->fs_relids, -1); - rte = rt_fetch(rtindex, estate->es_range_table); + rte = exec_rt_fetch(rtindex, estate->es_range_table_array); userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); /* Get info about foreign table. */ @@ -1731,8 +1731,8 @@ postgresBeginForeignModify(ModifyTableState *mtstate, FdwModifyPrivateRetrievedAttrs); /* Find RTE. */ - rte = rt_fetch(resultRelInfo->ri_RangeTableIndex, - mtstate->ps.state->es_range_table); + rte = exec_rt_fetch(resultRelInfo->ri_RangeTableIndex, + mtstate->ps.state->es_range_table_array); /* Construct an execution state. */ fmstate = create_foreign_modify(mtstate->ps.state, @@ -2036,7 +2036,7 @@ postgresBeginForeignInsert(ModifyTableState *mtstate, * correspond to this partition if it is one of the UPDATE subplan target * rels; in that case, we can just use the existing RTE as-is. */ - rte = list_nth(estate->es_range_table, resultRelation - 1); + rte = exec_rt_fetch(resultRelation, estate->es_range_table_array); if (rte->relid != RelationGetRelid(rel)) { rte = copyObject(rte); @@ -2396,7 +2396,7 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) * ExecCheckRTEPerms() does. */ rtindex = estate->es_result_relation_info->ri_RangeTableIndex; - rte = rt_fetch(rtindex, estate->es_range_table); + rte = exec_rt_fetch(rtindex, estate->es_range_table_array); userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); /* Get info about foreign table. */ @@ -5752,7 +5752,7 @@ conversion_error_callback(void *arg) RangeTblEntry *rte; Var *var = (Var *) tle->expr; - rte = rt_fetch(var->varno, estate->es_range_table); + rte = exec_rt_fetch(var->varno, estate->es_range_table_array); if (var->varattno == 0) is_wholerow = true; diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 32706fad90..b2fededc9d 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2484,7 +2484,7 @@ CopyFrom(CopyState cstate) estate->es_result_relations = resultRelInfo; estate->es_num_result_relations = 1; estate->es_result_relation_info = resultRelInfo; - estate->es_range_table = cstate->range_table; + ExecInitRangeTable(estate, cstate->range_table); /* Set up a tuple slot too */ myslot = ExecInitExtraTupleSlot(estate, tupDesc); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 136f9f0627..5ce3720440 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -75,7 +75,8 @@ static int MyTriggerDepth = 0; * to be changed, however. */ #define GetUpdatedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) + (exec_rt_fetch((relinfo)->ri_RangeTableIndex,\ + (estate)->es_range_table_array)->updatedCols) /* Local function prototypes */ static void ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid); diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index c549e3db5d..33cadf8e34 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -3934,10 +3934,11 @@ ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext) * perhaps other places.) */ if (econtext->ecxt_estate && - variable->varno <= list_length(econtext->ecxt_estate->es_range_table)) + variable->varno <= econtext->ecxt_estate->es_range_table_size) { - RangeTblEntry *rte = rt_fetch(variable->varno, - econtext->ecxt_estate->es_range_table); + RangeTblEntry *rte = + exec_rt_fetch(variable->varno, + econtext->ecxt_estate->es_range_table_array); if (rte->eref) ExecTypeSetColNames(output_tupdesc, rte->eref->colnames); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 4ec47ac41e..d5227ae129 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -109,9 +109,9 @@ static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, * to be changed, however. */ #define GetInsertedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->insertedCols) + (exec_rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table_array)->insertedCols) #define GetUpdatedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) + (exec_rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table_array)->updatedCols) /* end of local decls */ @@ -823,7 +823,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* * initialize the node's execution state */ - estate->es_range_table = rangeTable; + ExecInitRangeTable(estate, rangeTable); /* * Allocate an array to store open Relations of each rangeTable item. We @@ -918,9 +918,10 @@ InitPlan(QueryDesc *queryDesc, int eflags) resultRelIndex)) { Relation resultRelDesc; + Oid reloid = getrelid(resultRelIndex, + estate->es_range_table_array); - resultRelDesc = heap_open(getrelid(resultRelIndex, rangeTable), - NoLock); + resultRelDesc = heap_open(reloid, NoLock); Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); heap_close(resultRelDesc, NoLock); } @@ -962,7 +963,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) continue; /* get relation's OID (will produce InvalidOid if subquery) */ - relid = getrelid(rc->rti, rangeTable); + relid = getrelid(rc->rti, estate->es_range_table_array); switch (rc->markType) { @@ -3072,7 +3073,7 @@ EvalPlanQualBegin(EPQState *epqstate, EState *parentestate) /* * We already have a suitable child EPQ tree, so just reset it. */ - int rtsize = list_length(parentestate->es_range_table); + int rtsize = parentestate->es_range_table_size; PlanState *planstate = epqstate->planstate; MemSet(estate->es_epqScanDone, 0, rtsize * sizeof(bool)); @@ -3125,7 +3126,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) MemoryContext oldcontext; ListCell *l; - rtsize = list_length(parentestate->es_range_table); + rtsize = parentestate->es_range_table_size; epqstate->estate = estate = CreateExecutorState(); @@ -3149,6 +3150,8 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) estate->es_snapshot = parentestate->es_snapshot; estate->es_crosscheck_snapshot = parentestate->es_crosscheck_snapshot; estate->es_range_table = parentestate->es_range_table; + estate->es_range_table_array = parentestate->es_range_table_array; + estate->es_range_table_size = parentestate->es_range_table_size; estate->es_relations = parentestate->es_relations; estate->es_plannedstmt = parentestate->es_plannedstmt; estate->es_junkFilter = parentestate->es_junkFilter; diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 4825756a32..2ff36ef364 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -34,6 +34,8 @@ * GetAttributeByName Runtime extraction of columns from tuples. * GetAttributeByNum * + * ExecInitRangeTable Build an array from the range table list to + * allow faster lookups by relid. * NOTES * This file has traditionally been the place to stick misc. * executor support stuff that doesn't really go anyplace else. @@ -837,7 +839,7 @@ ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate) ListCell *l; Index rti = lfirst_int(lc); bool is_result_rel = false; - Oid relid = getrelid(rti, estate->es_range_table); + Oid relid = getrelid(rti, estate->es_range_table_array); /* If this is a result relation, already locked in InitPlan */ foreach(l, stmt->nonleafResultRelations) @@ -1031,6 +1033,26 @@ ExecCleanTargetListLength(List *targetlist) } /* + * Initialize executor's range table array + */ +void +ExecInitRangeTable(EState *estate, List *rangeTable) +{ + int rti; + ListCell *l; + + Assert(estate != NULL); + estate->es_range_table = rangeTable; + estate->es_range_table_size = list_length(rangeTable); + estate->es_range_table_array = (RangeTblEntry **) + palloc(estate->es_range_table_size * + sizeof(RangeTblEntry *)); + rti = 0; + foreach(l, rangeTable) + estate->es_range_table_array[rti++] = lfirst_node(RangeTblEntry, l); +} + +/* * ExecRangeTableRelation * Open and lock, if not already done, a range table relation */ diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 30de8a95ab..6db345ae7a 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -400,7 +400,7 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags) /* * Create workspace in which we can remember per-RTE locked tuples */ - lrstate->lr_ntables = list_length(estate->es_range_table); + lrstate->lr_ntables = estate->es_range_table_size; lrstate->lr_curtuples = (HeapTuple *) palloc0(lrstate->lr_ntables * sizeof(HeapTuple)); diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index e247539d7b..b54e427be4 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -201,6 +201,8 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel) rte->relkind = rel->localrel->rd_rel->relkind; rte->rellockmode = AccessShareLock; estate->es_range_table = list_make1(rte); + estate->es_range_table_array = &rte; + estate->es_range_table_size = 1; resultRelInfo = makeNode(ResultRelInfo); InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index f3df6408a9..fdfd48d897 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -478,6 +478,7 @@ extern ExprContext *CreateExprContext(EState *estate); extern ExprContext *CreateStandaloneExprContext(void); extern void FreeExprContext(ExprContext *econtext, bool isCommit); extern void ReScanExprContext(ExprContext *econtext); +extern void ExecInitRangeTable(EState *estate, List *rangeTable); #define ResetExprContext(econtext) \ MemoryContextReset((econtext)->ecxt_per_tuple_memory) diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 7c47967494..fcdb7118a6 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -20,6 +20,7 @@ #include "executor/instrument.h" #include "lib/pairingheap.h" #include "nodes/params.h" +#include "nodes/parsenodes.h" #include "nodes/plannodes.h" #include "utils/hsearch.h" #include "utils/queryenvironment.h" @@ -479,6 +480,8 @@ typedef struct EState Snapshot es_snapshot; /* time qual to use */ Snapshot es_crosscheck_snapshot; /* crosscheck time qual for RI */ List *es_range_table; /* List of RangeTblEntry */ + struct RangeTblEntry **es_range_table_array; /* 0-based range table */ + int es_range_table_size; /* size of the range table array */ Relation *es_relations; /* 0-based array of Relations * es_range_table_size elements in length. */ PlannedStmt *es_plannedstmt; /* link to top of plan tree */ @@ -581,6 +584,22 @@ typedef struct EState struct JitInstrumentation *es_jit_worker_instr; } EState; +/* + * exec_rt_fetch + * + * NB: this will crash and burn if handed an out-of-range RT index + */ +#define exec_rt_fetch(rangetblidx, rangetbl) rangetbl[(rangetblidx) - 1] + +/* + * getrelid + * + * Given the range index of a relation, return the corresponding + * relation OID. Note that InvalidOid will be returned if the + * RTE is for a non-relation-type RTE. + */ +#define getrelid(rangeindex,rangetable) \ + (exec_rt_fetch(rangeindex, rangetable)->relid) /* * ExecRowMark - diff --git a/src/include/parser/parsetree.h b/src/include/parser/parsetree.h index dd9ae658ac..fe16d7d1fa 100644 --- a/src/include/parser/parsetree.h +++ b/src/include/parser/parsetree.h @@ -32,16 +32,6 @@ ((RangeTblEntry *) list_nth(rangetable, (rangetable_index)-1)) /* - * getrelid - * - * Given the range index of a relation, return the corresponding - * relation OID. Note that InvalidOid will be returned if the - * RTE is for a non-relation-type RTE. - */ -#define getrelid(rangeindex,rangetable) \ - (rt_fetch(rangeindex, rangetable)->relid) - -/* * Given an RTE and an attribute number, return the appropriate * variable name or alias for that attribute of that RTE. */ -- 2.11.0
From fadc610644cfa352485e2ee8134640b7954f4cef Mon Sep 17 00:00:00 2001 From: amit <amitlangot...@gmail.com> Date: Thu, 19 Jul 2018 16:14:51 +0900 Subject: [PATCH v12 3/5] Move result rel and ExecRowMark initilization to ExecInitNode InitPlan would do certain initiaization steps, such as creating result rels, ExecRowMarks, etc. only because of the concerns about locking order, which it needs not to do anymore, because we've eliminated executor locking at all. Instead create result rels and ExecRowMarks, etc. in the ExecInit* routines of the respective nodes. --- src/backend/executor/execMain.c | 238 +++++++++++---------------------- src/backend/executor/nodeLockRows.c | 4 +- src/backend/executor/nodeModifyTable.c | 27 +++- src/include/executor/executor.h | 2 +- 4 files changed, 103 insertions(+), 168 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index d5227ae129..0e77c3b66f 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -836,98 +836,44 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_plannedstmt = plannedstmt; /* - * initialize result relation stuff, and open/lock the result rels. - * - * We must do this before initializing the plan tree, else we might try to - * do a lock upgrade if a result rel is also a source rel. + * Allocate memory required for result relations. ResultRelInfos + * themselves are initialized during ExecInitModifyTable of respective + * ModifyTable nodes. */ if (plannedstmt->resultRelations) { - List *resultRelations = plannedstmt->resultRelations; - int numResultRelations = list_length(resultRelations); - ResultRelInfo *resultRelInfos; - ResultRelInfo *resultRelInfo; + int numResultRelations = list_length(plannedstmt->resultRelations); + int num_roots = list_length(plannedstmt->rootResultRelations); - resultRelInfos = (ResultRelInfo *) - palloc(numResultRelations * sizeof(ResultRelInfo)); - resultRelInfo = resultRelInfos; - foreach(l, resultRelations) - { - Index resultRelationIndex = lfirst_int(l); - Relation resultRelation; - - Assert(rt_fetch(resultRelationIndex, rangeTable)->rellockmode == RowExclusiveLock); - resultRelation = ExecRangeTableRelation(estate, resultRelationIndex); - InitResultRelInfo(resultRelInfo, - resultRelation, - resultRelationIndex, - NULL, - estate->es_instrument); - resultRelInfo++; - } - estate->es_result_relations = resultRelInfos; + estate->es_result_relations = (ResultRelInfo *) + palloc(numResultRelations * sizeof(ResultRelInfo)); estate->es_num_result_relations = numResultRelations; /* es_result_relation_info is NULL except when within ModifyTable */ estate->es_result_relation_info = NULL; - /* - * In the partitioned result relation case, lock the non-leaf result - * relations too. A subset of these are the roots of respective - * partitioned tables, for which we also allocate ResultRelInfos. - */ - estate->es_root_result_relations = NULL; - estate->es_num_root_result_relations = 0; - if (plannedstmt->nonleafResultRelations) - { - int num_roots = list_length(plannedstmt->rootResultRelations); - - /* - * Firstly, build ResultRelInfos for all the partitioned table - * roots, because we will need them to fire the statement-level - * triggers, if any. - */ - resultRelInfos = (ResultRelInfo *) + /* For partitioned tables that are roots of their partition trees. */ + estate->es_root_result_relations = (ResultRelInfo *) palloc(num_roots * sizeof(ResultRelInfo)); - resultRelInfo = resultRelInfos; - foreach(l, plannedstmt->rootResultRelations) - { - Index resultRelIndex = lfirst_int(l); - Relation resultRelDesc; + estate->es_num_root_result_relations = num_roots; - Assert(rt_fetch(resultRelIndex, rangeTable)->rellockmode == RowExclusiveLock); - resultRelDesc = ExecRangeTableRelation(estate, resultRelIndex); - InitResultRelInfo(resultRelInfo, - resultRelDesc, - lfirst_int(l), - NULL, - estate->es_instrument); - resultRelInfo++; - } - - estate->es_root_result_relations = resultRelInfos; - estate->es_num_root_result_relations = num_roots; - - /* Simply check the rest of them are locked. */ + /* + * Since non-leaf tables of partition trees won't be processed by + * ExecInitModifyTable, so check here that the appropriate lock is + * held by upstream. + */ #ifdef USE_ASSERT_CHECKING - foreach(l, plannedstmt->nonleafResultRelations) - { - Index resultRelIndex = lfirst_int(l); + foreach(l, plannedstmt->nonleafResultRelations) + { + Index resultRelIndex = lfirst_int(l); + Relation resultRelDesc; + Oid reloid = getrelid(resultRelIndex, + estate->es_range_table_array); - /* We locked the roots above. */ - if (!list_member_int(plannedstmt->rootResultRelations, - resultRelIndex)) - { - Relation resultRelDesc; - Oid reloid = getrelid(resultRelIndex, - estate->es_range_table_array); - - resultRelDesc = heap_open(reloid, NoLock); - Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); - heap_close(resultRelDesc, NoLock); - } - } -#endif + resultRelDesc = heap_open(reloid, NoLock); + Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); + heap_close(resultRelDesc, NoLock); } +#endif } else { @@ -941,67 +887,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_num_root_result_relations = 0; } - /* - * Similarly, we have to lock relations selected FOR [KEY] UPDATE/SHARE - * before we initialize the plan tree, else we'd be risking lock upgrades. - * While we are at it, build the ExecRowMark list. Any partitioned child - * tables are ignored here (because isParent=true) and will be locked by - * the first Append or MergeAppend node that references them. (Note that - * the RowMarks corresponding to partitioned child tables are present in - * the same list as the rest, i.e., plannedstmt->rowMarks.) - */ estate->es_rowMarks = NIL; - foreach(l, plannedstmt->rowMarks) - { - PlanRowMark *rc = (PlanRowMark *) lfirst(l); - Oid relid; - Relation relation; - ExecRowMark *erm; - - /* ignore "parent" rowmarks; they are irrelevant at runtime */ - if (rc->isParent) - continue; - - /* get relation's OID (will produce InvalidOid if subquery) */ - relid = getrelid(rc->rti, estate->es_range_table_array); - - switch (rc->markType) - { - case ROW_MARK_EXCLUSIVE: - case ROW_MARK_NOKEYEXCLUSIVE: - case ROW_MARK_SHARE: - case ROW_MARK_KEYSHARE: - case ROW_MARK_REFERENCE: - relation = ExecRangeTableRelation(estate, rc->rti); - break; - case ROW_MARK_COPY: - /* no physical table access is required */ - relation = NULL; - break; - default: - elog(ERROR, "unrecognized markType: %d", rc->markType); - relation = NULL; /* keep compiler quiet */ - break; - } - - /* Check that relation is a legal target for marking */ - if (relation) - CheckValidRowMarkRel(relation, rc->markType); - - erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); - erm->relation = relation; - erm->relid = relid; - erm->rti = rc->rti; - erm->prti = rc->prti; - erm->rowmarkId = rc->rowmarkId; - erm->markType = rc->markType; - erm->strength = rc->strength; - erm->waitPolicy = rc->waitPolicy; - erm->ermActive = false; - ItemPointerSetInvalid(&(erm->curCtid)); - erm->ermExtra = NULL; - estate->es_rowMarks = lappend(estate->es_rowMarks, erm); - } /* * Initialize the executor's tuple table to empty. @@ -1609,7 +1495,6 @@ static void ExecEndPlan(PlanState *planstate, EState *estate) { int num_relations = list_length(estate->es_range_table); - ResultRelInfo *resultRelInfo; int i; ListCell *l; @@ -1643,17 +1528,6 @@ ExecEndPlan(PlanState *planstate, EState *estate) */ ExecResetTupleTable(estate->es_tupleTable, false); - /* - * close indexes of result relation(s) if any - */ - resultRelInfo = estate->es_result_relations; - for (i = estate->es_num_result_relations; i > 0; i--) - { - /* Close indices; the relation itself already closed above */ - ExecCloseIndices(resultRelInfo); - resultRelInfo++; - } - /* likewise close any trigger target relations */ ExecCleanUpTriggerState(estate); } @@ -2409,25 +2283,63 @@ ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo) } /* - * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index + * ExecBuildRowMark -- create and return an ExecRowMark struct for the given + * PlanRowMark. * - * If no such struct, either return NULL or throw error depending on missing_ok + * The created ExecRowMark is also added to the list of rowmarks in the given + * EState. */ ExecRowMark * -ExecFindRowMark(EState *estate, Index rti, bool missing_ok) +ExecBuildRowMark(EState *estate, PlanRowMark *rc) { - ListCell *lc; + Oid relid; + ExecRowMark *erm; + Relation relation; - foreach(lc, estate->es_rowMarks) + Assert(rc != NULL && rc->rti > 0); + Assert(!rc->isParent); + relid = getrelid(rc->rti, estate->es_range_table_array); + + /* get relation's OID (will produce InvalidOid if subquery) */ + + switch (rc->markType) { - ExecRowMark *erm = (ExecRowMark *) lfirst(lc); - - if (erm->rti == rti) - return erm; + case ROW_MARK_EXCLUSIVE: + case ROW_MARK_NOKEYEXCLUSIVE: + case ROW_MARK_SHARE: + case ROW_MARK_KEYSHARE: + case ROW_MARK_REFERENCE: + relation = ExecRangeTableRelation(estate, rc->rti); + break; + case ROW_MARK_COPY: + /* no physical table access is required */ + relation = NULL; + break; + default: + elog(ERROR, "unrecognized markType: %d", rc->markType); + relation = NULL; /* keep compiler quiet */ + break; } - if (!missing_ok) - elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti); - return NULL; + + /* Check that relation is a legal target for marking */ + if (relation) + CheckValidRowMarkRel(relation, rc->markType); + + erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); + erm->relation = relation; + erm->relid = relid; + erm->rti = rc->rti; + erm->prti = rc->prti; + erm->rowmarkId = rc->rowmarkId; + erm->markType = rc->markType; + erm->strength = rc->strength; + erm->waitPolicy = rc->waitPolicy; + erm->ermActive = false; + ItemPointerSetInvalid(&(erm->curCtid)); + erm->ermExtra = NULL; + estate->es_rowMarks = lappend(estate->es_rowMarks, erm); + + return erm; } /* diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 6db345ae7a..ff9606342b 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -424,8 +424,8 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags) /* safety check on size of lr_curtuples array */ Assert(rc->rti > 0 && rc->rti <= lrstate->lr_ntables); - /* find ExecRowMark and build ExecAuxRowMark */ - erm = ExecFindRowMark(estate, rc->rti, false); + /* build the ExecRowMark and ExecAuxRowMark */ + erm = ExecBuildRowMark(estate, rc); aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist); /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 07ac3de5e0..9b584097d7 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2242,12 +2242,33 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans); mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex; + resultRelInfo = mtstate->resultRelInfo; + foreach(l, node->resultRelations) + { + Index resultRelationIndex = lfirst_int(l); + Relation rel; + + rel = ExecRangeTableRelation(estate, resultRelationIndex); + InitResultRelInfo(resultRelInfo, rel, resultRelationIndex, NULL, + estate->es_instrument); + resultRelInfo++; + } /* If modifying a partitioned table, initialize the root table info */ if (node->rootResultRelIndex >= 0) + { + Index root_rt_index = linitial_int(node->partitioned_rels); + Relation rel; + mtstate->rootResultRelInfo = estate->es_root_result_relations + node->rootResultRelIndex; + Assert(root_rt_index > 0); + rel = ExecRangeTableRelation(estate, root_rt_index); + InitResultRelInfo(mtstate->rootResultRelInfo, rel, root_rt_index, + NULL, estate->es_instrument); + } + mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans); mtstate->mt_nplans = nplans; @@ -2533,8 +2554,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (rc->isParent) continue; - /* find ExecRowMark (same for all subplans) */ - erm = ExecFindRowMark(estate, rc->rti, false); + /* build the ExecRowMark (same for all subplans) */ + erm = ExecBuildRowMark(estate, rc); /* build ExecAuxRowMark for each subplan */ for (i = 0; i < nplans; i++) @@ -2700,6 +2721,8 @@ ExecEndModifyTable(ModifyTableState *node) resultRelInfo->ri_FdwRoutine->EndForeignModify != NULL) resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state, resultRelInfo); + /* closes indices */ + ExecCloseIndices(resultRelInfo); } /* Close all the partitioned tables, leaf partitions, and their indices */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index fdfd48d897..4c3a78b1a7 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -188,7 +188,7 @@ extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo); -extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok); +extern ExecRowMark *ExecBuildRowMark(EState *estate, PlanRowMark *rc); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, Relation relation, Index rti, int lockmode, -- 2.11.0
From d62e0b3e05dbb156b9d66254cd1279fdd5ed8e34 Mon Sep 17 00:00:00 2001 From: "dgrow...@gmail.com" <dgrow...@gmail.com> Date: Thu, 27 Sep 2018 13:27:24 +1200 Subject: [PATCH v12 4/5] Remove useless fields from planner nodes They were added so that executor could identify range table entries entries to lock them or to impose order on locking during executor initialization, but turns out that executor doesn't take any locks of its own on the relations, so these fields are redundant. Following fields are removed: * 'partitioned_rels' from Append, MergeAppend, and ModifyTable. Actually, in ModifyTable, it is replaced by a bool field called 'partitionedTarget', which is set if the target is a partitioned table. The table itself is identified by 'targetRelation' which replaces 'nominalRelation'. * 'nonleafResultRelations' from PlannedStmt and PlannerGlobal. * 'rowMarks' from PlannedStmt and 'finalrowmarks' from PlannerGlobal. In PlannedStmt, gets a new bool hasRowMarks that stores if query->rowMarks != NIL. --- contrib/postgres_fdw/postgres_fdw.c | 2 +- src/backend/commands/explain.c | 6 +-- src/backend/commands/portalcmds.c | 2 +- src/backend/executor/execMain.c | 21 +------- src/backend/executor/execParallel.c | 3 +- src/backend/executor/execPartition.c | 2 +- src/backend/executor/execUtils.c | 60 ---------------------- src/backend/executor/nodeAppend.c | 6 --- src/backend/executor/nodeMergeAppend.c | 6 --- src/backend/executor/nodeModifyTable.c | 10 ++-- src/backend/executor/spi.c | 4 +- src/backend/nodes/copyfuncs.c | 9 ++-- src/backend/nodes/outfuncs.c | 15 ++---- src/backend/nodes/readfuncs.c | 9 ++-- src/backend/optimizer/plan/createplan.c | 46 ++++------------- src/backend/optimizer/plan/planner.c | 89 +++++++++------------------------ src/backend/optimizer/plan/setrefs.c | 49 ++---------------- src/backend/optimizer/util/pathnode.c | 12 ++--- src/backend/tcop/utility.c | 15 ++++-- src/include/executor/executor.h | 2 - src/include/nodes/plannodes.h | 30 ++++------- src/include/nodes/relation.h | 10 ++-- src/include/optimizer/pathnode.h | 2 +- 23 files changed, 96 insertions(+), 314 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 3dd1bab2a2..d20c75367c 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2050,7 +2050,7 @@ postgresBeginForeignInsert(ModifyTableState *mtstate, * Vars contained in those expressions. */ if (plan && plan->operation == CMD_UPDATE && - resultRelation == plan->nominalRelation) + resultRelation == plan->targetRelation) resultRelation = mtstate->resultRelInfo[0].ri_RangeTableIndex; } diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 799a22e9d5..a7a851a405 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -954,7 +954,7 @@ ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used) break; case T_ModifyTable: *rels_used = bms_add_member(*rels_used, - ((ModifyTable *) plan)->nominalRelation); + ((ModifyTable *) plan)->targetRelation); if (((ModifyTable *) plan)->exclRelRTI) *rels_used = bms_add_member(*rels_used, ((ModifyTable *) plan)->exclRelRTI); @@ -2949,7 +2949,7 @@ ExplainScanTarget(Scan *plan, ExplainState *es) static void ExplainModifyTarget(ModifyTable *plan, ExplainState *es) { - ExplainTargetRel((Plan *) plan, plan->nominalRelation, es); + ExplainTargetRel((Plan *) plan, plan->targetRelation, es); } /* @@ -3113,7 +3113,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors, /* Should we explicitly label target relations? */ labeltargets = (mtstate->mt_nplans > 1 || (mtstate->mt_nplans == 1 && - mtstate->resultRelInfo->ri_RangeTableIndex != node->nominalRelation)); + mtstate->resultRelInfo->ri_RangeTableIndex != node->targetRelation)); if (labeltargets) ExplainOpenGroup("Target Tables", "Target Tables", false, es); diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 568499761f..4b213a8db7 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -133,7 +133,7 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params, portal->cursorOptions = cstmt->options; if (!(portal->cursorOptions & (CURSOR_OPT_SCROLL | CURSOR_OPT_NO_SCROLL))) { - if (plan->rowMarks == NIL && + if (!plan->hasRowMarks && ExecSupportsBackwardScan(plan->planTree)) portal->cursorOptions |= CURSOR_OPT_SCROLL; else diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 0e77c3b66f..1288717b0a 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -217,7 +217,7 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags) * SELECT FOR [KEY] UPDATE/SHARE and modifying CTEs need to mark * tuples */ - if (queryDesc->plannedstmt->rowMarks != NIL || + if (queryDesc->plannedstmt->hasRowMarks || queryDesc->plannedstmt->hasModifyingCTE) estate->es_output_cid = GetCurrentCommandId(true); @@ -855,25 +855,6 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_root_result_relations = (ResultRelInfo *) palloc(num_roots * sizeof(ResultRelInfo)); estate->es_num_root_result_relations = num_roots; - - /* - * Since non-leaf tables of partition trees won't be processed by - * ExecInitModifyTable, so check here that the appropriate lock is - * held by upstream. - */ -#ifdef USE_ASSERT_CHECKING - foreach(l, plannedstmt->nonleafResultRelations) - { - Index resultRelIndex = lfirst_int(l); - Relation resultRelDesc; - Oid reloid = getrelid(resultRelIndex, - estate->es_range_table_array); - - resultRelDesc = heap_open(reloid, NoLock); - Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true)); - heap_close(resultRelDesc, NoLock); - } -#endif } else { diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 0fdbd119d9..aff14507e1 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -176,6 +176,7 @@ ExecSerializePlan(Plan *plan, EState *estate) pstmt->queryId = UINT64CONST(0); pstmt->hasReturning = false; pstmt->hasModifyingCTE = false; + pstmt->hasRowMarks = false; pstmt->canSetTag = true; pstmt->transientPlan = false; pstmt->dependsOnRole = false; @@ -183,7 +184,6 @@ ExecSerializePlan(Plan *plan, EState *estate) pstmt->planTree = plan; pstmt->rtable = estate->es_range_table; pstmt->resultRelations = NIL; - pstmt->nonleafResultRelations = NIL; /* * Transfer only parallel-safe subplans, leaving a NULL "hole" in the list @@ -203,7 +203,6 @@ ExecSerializePlan(Plan *plan, EState *estate) } pstmt->rewindPlanIDs = NULL; - pstmt->rowMarks = NIL; pstmt->relationOids = NIL; pstmt->invalItems = NIL; /* workers can't replan anyway... */ pstmt->paramExecTypes = estate->es_plannedstmt->paramExecTypes; diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index c082bb7632..5d9771c3a1 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -355,7 +355,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, leaf_part_rri = makeNode(ResultRelInfo); InitResultRelInfo(leaf_part_rri, partrel, - node ? node->nominalRelation : 1, + node ? node->targetRelation : 1, rootrel, estate->es_instrument); diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 2ff36ef364..cc895ec5a1 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -822,66 +822,6 @@ ShutdownExprContext(ExprContext *econtext, bool isCommit) } /* - * ExecLockNonLeafAppendTables - * - * Locks, if necessary, the tables indicated by the RT indexes contained in - * the partitioned_rels list. These are the non-leaf tables in the partition - * tree controlled by a given Append or MergeAppend node. - */ -void -ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate) -{ - PlannedStmt *stmt = estate->es_plannedstmt; - ListCell *lc; - - foreach(lc, partitioned_rels) - { - ListCell *l; - Index rti = lfirst_int(lc); - bool is_result_rel = false; - Oid relid = getrelid(rti, estate->es_range_table_array); - - /* If this is a result relation, already locked in InitPlan */ - foreach(l, stmt->nonleafResultRelations) - { - if (rti == lfirst_int(l)) - { - is_result_rel = true; - break; - } - } - - /* - * Not a result relation; check if there is a RowMark that requires - * taking a RowShareLock on this rel. - */ - if (!is_result_rel) - { - PlanRowMark *rc = NULL; - LOCKMODE lockmode; - - foreach(l, stmt->rowMarks) - { - if (((PlanRowMark *) lfirst(l))->rti == rti) - { - rc = lfirst(l); - break; - } - } - - if (rc && RowMarkRequiresRowShareLock(rc->markType)) - lockmode = RowShareLock; - else - lockmode = AccessShareLock; - - Assert(lockmode == rt_fetch(rti, estate->es_range_table)->rellockmode); - - LockRelationOid(relid, lockmode); - } - } -} - -/* * GetAttributeByName * GetAttributeByNum * diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index d44befd44e..a16b6da474 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -113,12 +113,6 @@ ExecInitAppend(Append *node, EState *estate, int eflags) Assert(!(eflags & EXEC_FLAG_MARK)); /* - * Lock the non-leaf tables in the partition tree controlled by this node. - * It's a no-op for non-partitioned parent tables. - */ - ExecLockNonLeafAppendTables(node->partitioned_rels, estate); - - /* * create new AppendState for our append node */ appendstate->ps.plan = (Plan *) node; diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c index 6daf60a454..5d0dbb8146 100644 --- a/src/backend/executor/nodeMergeAppend.c +++ b/src/backend/executor/nodeMergeAppend.c @@ -76,12 +76,6 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags) Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); /* - * Lock the non-leaf tables in the partition tree controlled by this node. - * It's a no-op for non-partitioned parent tables. - */ - ExecLockNonLeafAppendTables(node->partitioned_rels, estate); - - /* * create new MergeAppendState for our node */ mergestate->ps.plan = (Plan *) node; diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 9b584097d7..60d4bea7b3 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2257,16 +2257,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) /* If modifying a partitioned table, initialize the root table info */ if (node->rootResultRelIndex >= 0) { - Index root_rt_index = linitial_int(node->partitioned_rels); Relation rel; + Assert(node->partitionedTarget); + mtstate->rootResultRelInfo = estate->es_root_result_relations + node->rootResultRelIndex; - Assert(root_rt_index > 0); - rel = ExecRangeTableRelation(estate, root_rt_index); - InitResultRelInfo(mtstate->rootResultRelInfo, rel, root_rt_index, - NULL, estate->es_instrument); + rel = ExecRangeTableRelation(estate, node->targetRelation); + InitResultRelInfo(mtstate->rootResultRelInfo, rel, + node->targetRelation, NULL, estate->es_instrument); } mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans); diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 11ca800e4c..1bee240e72 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -1358,7 +1358,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, { if (list_length(stmt_list) == 1 && linitial_node(PlannedStmt, stmt_list)->commandType != CMD_UTILITY && - linitial_node(PlannedStmt, stmt_list)->rowMarks == NIL && + !linitial_node(PlannedStmt, stmt_list)->hasRowMarks && ExecSupportsBackwardScan(linitial_node(PlannedStmt, stmt_list)->planTree)) portal->cursorOptions |= CURSOR_OPT_SCROLL; else @@ -1374,7 +1374,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, { if (list_length(stmt_list) == 1 && linitial_node(PlannedStmt, stmt_list)->commandType != CMD_UTILITY && - linitial_node(PlannedStmt, stmt_list)->rowMarks != NIL) + linitial_node(PlannedStmt, stmt_list)->hasRowMarks) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("DECLARE SCROLL CURSOR ... FOR UPDATE/SHARE is not supported"), diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 3b690b55b3..da1820506a 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -83,6 +83,7 @@ _copyPlannedStmt(const PlannedStmt *from) COPY_SCALAR_FIELD(queryId); COPY_SCALAR_FIELD(hasReturning); COPY_SCALAR_FIELD(hasModifyingCTE); + COPY_SCALAR_FIELD(hasRowMarks); COPY_SCALAR_FIELD(canSetTag); COPY_SCALAR_FIELD(transientPlan); COPY_SCALAR_FIELD(dependsOnRole); @@ -91,11 +92,9 @@ _copyPlannedStmt(const PlannedStmt *from) COPY_NODE_FIELD(planTree); COPY_NODE_FIELD(rtable); COPY_NODE_FIELD(resultRelations); - COPY_NODE_FIELD(nonleafResultRelations); COPY_NODE_FIELD(rootResultRelations); COPY_NODE_FIELD(subplans); COPY_BITMAPSET_FIELD(rewindPlanIDs); - COPY_NODE_FIELD(rowMarks); COPY_NODE_FIELD(relationOids); COPY_NODE_FIELD(invalItems); COPY_NODE_FIELD(paramExecTypes); @@ -203,8 +202,8 @@ _copyModifyTable(const ModifyTable *from) */ COPY_SCALAR_FIELD(operation); COPY_SCALAR_FIELD(canSetTag); - COPY_SCALAR_FIELD(nominalRelation); - COPY_NODE_FIELD(partitioned_rels); + COPY_SCALAR_FIELD(targetRelation); + COPY_SCALAR_FIELD(partitionedTarget); COPY_SCALAR_FIELD(partColsUpdated); COPY_NODE_FIELD(resultRelations); COPY_SCALAR_FIELD(resultRelIndex); @@ -244,7 +243,6 @@ _copyAppend(const Append *from) */ COPY_NODE_FIELD(appendplans); COPY_SCALAR_FIELD(first_partial_plan); - COPY_NODE_FIELD(partitioned_rels); COPY_NODE_FIELD(part_prune_info); return newnode; @@ -266,7 +264,6 @@ _copyMergeAppend(const MergeAppend *from) /* * copy remainder of node */ - COPY_NODE_FIELD(partitioned_rels); COPY_NODE_FIELD(mergeplans); COPY_SCALAR_FIELD(numCols); COPY_POINTER_FIELD(sortColIdx, from->numCols * sizeof(AttrNumber)); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 863d29cc57..6d9ccd50d8 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -272,6 +272,7 @@ _outPlannedStmt(StringInfo str, const PlannedStmt *node) WRITE_UINT64_FIELD(queryId); WRITE_BOOL_FIELD(hasReturning); WRITE_BOOL_FIELD(hasModifyingCTE); + WRITE_BOOL_FIELD(hasRowMarks); WRITE_BOOL_FIELD(canSetTag); WRITE_BOOL_FIELD(transientPlan); WRITE_BOOL_FIELD(dependsOnRole); @@ -280,11 +281,9 @@ _outPlannedStmt(StringInfo str, const PlannedStmt *node) WRITE_NODE_FIELD(planTree); WRITE_NODE_FIELD(rtable); WRITE_NODE_FIELD(resultRelations); - WRITE_NODE_FIELD(nonleafResultRelations); WRITE_NODE_FIELD(rootResultRelations); WRITE_NODE_FIELD(subplans); WRITE_BITMAPSET_FIELD(rewindPlanIDs); - WRITE_NODE_FIELD(rowMarks); WRITE_NODE_FIELD(relationOids); WRITE_NODE_FIELD(invalItems); WRITE_NODE_FIELD(paramExecTypes); @@ -375,8 +374,8 @@ _outModifyTable(StringInfo str, const ModifyTable *node) WRITE_ENUM_FIELD(operation, CmdType); WRITE_BOOL_FIELD(canSetTag); - WRITE_UINT_FIELD(nominalRelation); - WRITE_NODE_FIELD(partitioned_rels); + WRITE_UINT_FIELD(targetRelation); + WRITE_BOOL_FIELD(partitionedTarget); WRITE_BOOL_FIELD(partColsUpdated); WRITE_NODE_FIELD(resultRelations); WRITE_INT_FIELD(resultRelIndex); @@ -405,7 +404,6 @@ _outAppend(StringInfo str, const Append *node) WRITE_NODE_FIELD(appendplans); WRITE_INT_FIELD(first_partial_plan); - WRITE_NODE_FIELD(partitioned_rels); WRITE_NODE_FIELD(part_prune_info); } @@ -418,7 +416,6 @@ _outMergeAppend(StringInfo str, const MergeAppend *node) _outPlanInfo(str, (const Plan *) node); - WRITE_NODE_FIELD(partitioned_rels); WRITE_NODE_FIELD(mergeplans); WRITE_INT_FIELD(numCols); @@ -2178,8 +2175,8 @@ _outModifyTablePath(StringInfo str, const ModifyTablePath *node) WRITE_ENUM_FIELD(operation, CmdType); WRITE_BOOL_FIELD(canSetTag); - WRITE_UINT_FIELD(nominalRelation); - WRITE_NODE_FIELD(partitioned_rels); + WRITE_UINT_FIELD(targetRelation); + WRITE_BOOL_FIELD(partitionedTarget); WRITE_BOOL_FIELD(partColsUpdated); WRITE_NODE_FIELD(resultRelations); WRITE_NODE_FIELD(subpaths); @@ -2257,9 +2254,7 @@ _outPlannerGlobal(StringInfo str, const PlannerGlobal *node) WRITE_NODE_FIELD(subplans); WRITE_BITMAPSET_FIELD(rewindPlanIDs); WRITE_NODE_FIELD(finalrtable); - WRITE_NODE_FIELD(finalrowmarks); WRITE_NODE_FIELD(resultRelations); - WRITE_NODE_FIELD(nonleafResultRelations); WRITE_NODE_FIELD(rootResultRelations); WRITE_NODE_FIELD(relationOids); WRITE_NODE_FIELD(invalItems); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 73ffa9714c..e07f38538c 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1496,6 +1496,7 @@ _readPlannedStmt(void) READ_UINT64_FIELD(queryId); READ_BOOL_FIELD(hasReturning); READ_BOOL_FIELD(hasModifyingCTE); + READ_BOOL_FIELD(hasRowMarks); READ_BOOL_FIELD(canSetTag); READ_BOOL_FIELD(transientPlan); READ_BOOL_FIELD(dependsOnRole); @@ -1504,11 +1505,9 @@ _readPlannedStmt(void) READ_NODE_FIELD(planTree); READ_NODE_FIELD(rtable); READ_NODE_FIELD(resultRelations); - READ_NODE_FIELD(nonleafResultRelations); READ_NODE_FIELD(rootResultRelations); READ_NODE_FIELD(subplans); READ_BITMAPSET_FIELD(rewindPlanIDs); - READ_NODE_FIELD(rowMarks); READ_NODE_FIELD(relationOids); READ_NODE_FIELD(invalItems); READ_NODE_FIELD(paramExecTypes); @@ -1597,8 +1596,8 @@ _readModifyTable(void) READ_ENUM_FIELD(operation, CmdType); READ_BOOL_FIELD(canSetTag); - READ_UINT_FIELD(nominalRelation); - READ_NODE_FIELD(partitioned_rels); + READ_UINT_FIELD(targetRelation); + READ_BOOL_FIELD(partitionedTarget); READ_BOOL_FIELD(partColsUpdated); READ_NODE_FIELD(resultRelations); READ_INT_FIELD(resultRelIndex); @@ -1632,7 +1631,6 @@ _readAppend(void) READ_NODE_FIELD(appendplans); READ_INT_FIELD(first_partial_plan); - READ_NODE_FIELD(partitioned_rels); READ_NODE_FIELD(part_prune_info); READ_DONE(); @@ -1648,7 +1646,6 @@ _readMergeAppend(void) ReadCommonPlan(&local_node->plan); - READ_NODE_FIELD(partitioned_rels); READ_NODE_FIELD(mergeplans); READ_INT_FIELD(numCols); READ_ATTRNUMBER_ARRAY(sortColIdx, local_node->numCols); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index ae41c9efa0..8f20e54d9e 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -124,7 +124,6 @@ static BitmapHeapScan *create_bitmap_scan_plan(PlannerInfo *root, static Plan *create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, List **qual, List **indexqual, List **indexECs); static void bitmap_subplan_mark_shared(Plan *plan); -static List *flatten_partitioned_rels(List *partitioned_rels); static TidScan *create_tidscan_plan(PlannerInfo *root, TidPath *best_path, List *tlist, List *scan_clauses); static SubqueryScan *create_subqueryscan_plan(PlannerInfo *root, @@ -203,8 +202,7 @@ static NamedTuplestoreScan *make_namedtuplestorescan(List *qptlist, List *qpqual static WorkTableScan *make_worktablescan(List *qptlist, List *qpqual, Index scanrelid, int wtParam); static Append *make_append(List *appendplans, int first_partial_plan, - List *tlist, List *partitioned_rels, - PartitionPruneInfo *partpruneinfo); + List *tlist, PartitionPruneInfo *partpruneinfo); static RecursiveUnion *make_recursive_union(List *tlist, Plan *lefttree, Plan *righttree, @@ -280,7 +278,7 @@ static Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan); static ProjectSet *make_project_set(List *tlist, Plan *subplan); static ModifyTable *make_modifytable(PlannerInfo *root, CmdType operation, bool canSetTag, - Index nominalRelation, List *partitioned_rels, + Index targetRelation, bool partitionedTarget, bool partColsUpdated, List *resultRelations, List *subplans, List *subroots, List *withCheckOptionLists, List *returningLists, @@ -1110,8 +1108,7 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path) */ plan = make_append(subplans, best_path->first_partial_path, - tlist, best_path->partitioned_rels, - partpruneinfo); + tlist, partpruneinfo); copy_generic_path_info(&plan->plan, (Path *) best_path); @@ -1253,8 +1250,6 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path) prunequal); } - node->partitioned_rels = - flatten_partitioned_rels(best_path->partitioned_rels); node->mergeplans = subplans; node->part_prune_info = partpruneinfo; @@ -2410,8 +2405,8 @@ create_modifytable_plan(PlannerInfo *root, ModifyTablePath *best_path) plan = make_modifytable(root, best_path->operation, best_path->canSetTag, - best_path->nominalRelation, - best_path->partitioned_rels, + best_path->targetRelation, + best_path->partitionedTarget, best_path->partColsUpdated, best_path->resultRelations, subplans, @@ -5005,27 +5000,6 @@ bitmap_subplan_mark_shared(Plan *plan) elog(ERROR, "unrecognized node type: %d", nodeTag(plan)); } -/* - * flatten_partitioned_rels - * Convert List of Lists into a single List with all elements from the - * sub-lists. - */ -static List * -flatten_partitioned_rels(List *partitioned_rels) -{ - List *newlist = NIL; - ListCell *lc; - - foreach(lc, partitioned_rels) - { - List *sublist = lfirst(lc); - - newlist = list_concat(newlist, list_copy(sublist)); - } - - return newlist; -} - /***************************************************************************** * * PLAN NODE BUILDING ROUTINES @@ -5368,8 +5342,7 @@ make_foreignscan(List *qptlist, static Append * make_append(List *appendplans, int first_partial_plan, - List *tlist, List *partitioned_rels, - PartitionPruneInfo *partpruneinfo) + List *tlist, PartitionPruneInfo *partpruneinfo) { Append *node = makeNode(Append); Plan *plan = &node->plan; @@ -5380,7 +5353,6 @@ make_append(List *appendplans, int first_partial_plan, plan->righttree = NULL; node->appendplans = appendplans; node->first_partial_plan = first_partial_plan; - node->partitioned_rels = flatten_partitioned_rels(partitioned_rels); node->part_prune_info = partpruneinfo; return node; } @@ -6509,7 +6481,7 @@ make_project_set(List *tlist, static ModifyTable * make_modifytable(PlannerInfo *root, CmdType operation, bool canSetTag, - Index nominalRelation, List *partitioned_rels, + Index targetRelation, bool partitionedTarget, bool partColsUpdated, List *resultRelations, List *subplans, List *subroots, List *withCheckOptionLists, List *returningLists, @@ -6537,8 +6509,8 @@ make_modifytable(PlannerInfo *root, node->operation = operation; node->canSetTag = canSetTag; - node->nominalRelation = nominalRelation; - node->partitioned_rels = flatten_partitioned_rels(partitioned_rels); + node->targetRelation = targetRelation; + node->partitionedTarget = partitionedTarget; node->partColsUpdated = partColsUpdated; node->resultRelations = resultRelations; node->resultRelIndex = -1; /* will be set correctly in setrefs.c */ diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 89625f4f5b..6a5d2f9d99 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -287,6 +287,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) Plan *top_plan; ListCell *lp, *lr; + bool hasRowMarks = (parse->rowMarks != NIL); /* * Set up global state for this planner invocation. This data is needed @@ -301,9 +302,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) glob->subroots = NIL; glob->rewindPlanIDs = NULL; glob->finalrtable = NIL; - glob->finalrowmarks = NIL; glob->resultRelations = NIL; - glob->nonleafResultRelations = NIL; glob->rootResultRelations = NIL; glob->relationOids = NIL; glob->invalItems = NIL; @@ -501,9 +500,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) /* final cleanup of the plan */ Assert(glob->finalrtable == NIL); - Assert(glob->finalrowmarks == NIL); Assert(glob->resultRelations == NIL); - Assert(glob->nonleafResultRelations == NIL); Assert(glob->rootResultRelations == NIL); top_plan = set_plan_references(root, top_plan); /* ... and the subplans (both regular subplans and initplans) */ @@ -514,6 +511,8 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) PlannerInfo *subroot = lfirst_node(PlannerInfo, lr); lfirst(lp) = set_plan_references(subroot, subplan); + if (subroot->rowMarks != NIL) + hasRowMarks = true; } /* build the PlannedStmt result */ @@ -523,6 +522,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) result->queryId = parse->queryId; result->hasReturning = (parse->returningList != NIL); result->hasModifyingCTE = parse->hasModifyingCTE; + result->hasRowMarks = hasRowMarks; result->canSetTag = parse->canSetTag; result->transientPlan = glob->transientPlan; result->dependsOnRole = glob->dependsOnRole; @@ -530,11 +530,9 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) result->planTree = top_plan; result->rtable = glob->finalrtable; result->resultRelations = glob->resultRelations; - result->nonleafResultRelations = glob->nonleafResultRelations; result->rootResultRelations = glob->rootResultRelations; result->subplans = glob->subplans; result->rewindPlanIDs = glob->rewindPlanIDs; - result->rowMarks = glob->finalrowmarks; result->relationOids = glob->relationOids; result->invalItems = glob->invalItems; result->paramExecTypes = glob->paramExecTypes; @@ -1169,7 +1167,7 @@ inheritance_planner(PlannerInfo *root) int top_parentRTindex = parse->resultRelation; Bitmapset *subqueryRTindexes; Bitmapset *modifiableARIindexes; - int nominalRelation = -1; + int targetRelation = -1; List *final_rtable = NIL; int save_rel_array_size = 0; RelOptInfo **save_rel_array = NULL; @@ -1184,8 +1182,7 @@ inheritance_planner(PlannerInfo *root) ListCell *lc; Index rti; RangeTblEntry *parent_rte; - Relids partitioned_relids = NULL; - List *partitioned_rels = NIL; + bool partitionedTarget = false; PlannerInfo *parent_root; Query *parent_parse; Bitmapset *parent_relids = bms_make_singleton(top_parentRTindex); @@ -1248,25 +1245,14 @@ inheritance_planner(PlannerInfo *root) } /* - * If the parent RTE is a partitioned table, we should use that as the - * nominal relation, because the RTEs added for partitioned tables - * (including the root parent) as child members of the inheritance set do - * not appear anywhere else in the plan. The situation is exactly the - * opposite in the case of non-partitioned inheritance parent as described - * below. For the same reason, collect the list of descendant partitioned - * tables to be saved in ModifyTable node, so that executor can lock those - * as well. + * If the parent RTE is a partitioned table, set that as the target + * relation and mark that we're working with a partitioned target. */ parent_rte = rt_fetch(top_parentRTindex, root->parse->rtable); if (parent_rte->relkind == RELKIND_PARTITIONED_TABLE) { - nominalRelation = top_parentRTindex; - - /* - * Root parent's RT index is always present in the partitioned_rels of - * the ModifyTable node, if one is needed at all. - */ - partitioned_relids = bms_make_singleton(top_parentRTindex); + targetRelation = top_parentRTindex; + partitionedTarget = true; } /* @@ -1338,7 +1324,7 @@ inheritance_planner(PlannerInfo *root) * inheritance parent. */ subroot->inhTargetKind = - partitioned_relids ? INHKIND_PARTITIONED : INHKIND_INHERITED; + partitionedTarget ? INHKIND_PARTITIONED : INHKIND_INHERITED; /* * If this child is further partitioned, remember it as a parent. @@ -1363,17 +1349,17 @@ inheritance_planner(PlannerInfo *root) } /* - * Set the nominal target relation of the ModifyTable node if not - * already done. We use the inheritance parent RTE as the nominal - * target relation if it's a partitioned table (see just above this - * loop). In the non-partitioned parent case, we'll use the first - * child relation (even if it's excluded) as the nominal target - * relation. Because of the way expand_inherited_rtentry works, the - * latter should be the RTE representing the parent table in its role - * as a simple member of the inheritance set. + * Set the target relation of the ModifyTable node if not already + * done. We use the inheritance parent RTE as the target relation + * if it's a partitioned table (see just above this loop). In the + * non-partitioned parent case, we'll use the first child relation + * (even if it's excluded) as the target relation. Because of the way + * expand_inherited_rtentry works, the latter should be the RTE + * representing the parent table in its role as a simple member of + * the inheritance set. * * It would be logically cleaner to *always* use the inheritance - * parent RTE as the nominal relation; but that RTE is not otherwise + * parent RTE as the target relation; but that RTE is not otherwise * referenced in the plan in the non-partitioned inheritance case. * Instead the duplicate child RTE created by expand_inherited_rtentry * is used elsewhere in the plan, so using the original parent RTE @@ -1383,8 +1369,8 @@ inheritance_planner(PlannerInfo *root) * duplicate child RTE added for the parent does not appear anywhere * else in the plan tree. */ - if (nominalRelation < 0) - nominalRelation = appinfo->child_relid; + if (targetRelation < 0) + targetRelation = appinfo->child_relid; /* * The rowMarks list might contain references to subquery RTEs, so @@ -1509,15 +1495,6 @@ inheritance_planner(PlannerInfo *root) continue; /* - * Add the current parent's RT index to the partitioned_relids set if - * we're creating the ModifyTable path for a partitioned root table. - * (We only care about parents of non-excluded children.) - */ - if (partitioned_relids) - partitioned_relids = bms_add_member(partitioned_relids, - appinfo->parent_relid); - - /* * If this is the first non-excluded child, its post-planning rtable * becomes the initial contents of final_rtable; otherwise, append * just its modified subquery RTEs to final_rtable. @@ -1620,29 +1597,13 @@ inheritance_planner(PlannerInfo *root) else rowMarks = root->rowMarks; - if (partitioned_relids) - { - int i; - - i = -1; - while ((i = bms_next_member(partitioned_relids, i)) >= 0) - partitioned_rels = lappend_int(partitioned_rels, i); - - /* - * If we're going to create ModifyTable at all, the list should - * contain at least one member, that is, the root parent's index. - */ - Assert(list_length(partitioned_rels) >= 1); - partitioned_rels = list_make1(partitioned_rels); - } - /* Create Path representing a ModifyTable to do the UPDATE/DELETE work */ add_path(final_rel, (Path *) create_modifytable_path(root, final_rel, parse->commandType, parse->canSetTag, - nominalRelation, - partitioned_rels, + targetRelation, + partitionedTarget, root->partColsUpdated, resultRelations, subpaths, @@ -2219,7 +2180,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, parse->commandType, parse->canSetTag, parse->resultRelation, - NIL, + false, false, list_make1_int(parse->resultRelation), list_make1(path), diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 13ee458802..ee01ef82d2 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -211,7 +211,6 @@ set_plan_references(PlannerInfo *root, Plan *plan) { PlannerGlobal *glob = root->glob; int rtoffset = list_length(glob->finalrtable); - ListCell *lc; /* * Add all the query's RTEs to the flattened rangetable. The live ones @@ -220,25 +219,6 @@ set_plan_references(PlannerInfo *root, Plan *plan) */ add_rtes_to_flat_rtable(root, false); - /* - * Adjust RT indexes of PlanRowMarks and add to final rowmarks list - */ - foreach(lc, root->rowMarks) - { - PlanRowMark *rc = lfirst_node(PlanRowMark, lc); - PlanRowMark *newrc; - - /* flat copy is enough since all fields are scalars */ - newrc = (PlanRowMark *) palloc(sizeof(PlanRowMark)); - memcpy(newrc, rc, sizeof(PlanRowMark)); - - /* adjust indexes ... but *not* the rowmarkId */ - newrc->rti += rtoffset; - newrc->prti += rtoffset; - - glob->finalrowmarks = lappend(glob->finalrowmarks, newrc); - } - /* Now fix the Plan tree */ return set_plan_refs(root, plan, rtoffset); } @@ -847,13 +827,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) fix_scan_list(root, splan->exclRelTlist, rtoffset); } - splan->nominalRelation += rtoffset; + splan->targetRelation += rtoffset; splan->exclRelRTI += rtoffset; - foreach(l, splan->partitioned_rels) - { - lfirst_int(l) += rtoffset; - } foreach(l, splan->resultRelations) { lfirst_int(l) += rtoffset; @@ -884,24 +860,17 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) list_copy(splan->resultRelations)); /* - * If the main target relation is a partitioned table, the - * following list contains the RT indexes of partitioned child - * relations including the root, which are not included in the - * above list. We also keep RT indexes of the roots - * separately to be identified as such during the executor - * initialization. + * If the main target relation is a partitioned table, remember + * the root table's RT index. */ - if (splan->partitioned_rels != NIL) + if (splan->partitionedTarget) { - root->glob->nonleafResultRelations = - list_concat(root->glob->nonleafResultRelations, - list_copy(splan->partitioned_rels)); /* Remember where this root will be in the global list. */ splan->rootResultRelIndex = list_length(root->glob->rootResultRelations); root->glob->rootResultRelations = lappend_int(root->glob->rootResultRelations, - linitial_int(splan->partitioned_rels)); + splan->targetRelation); } } break; @@ -915,10 +884,6 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) */ set_dummy_tlist_references(plan, rtoffset); Assert(splan->plan.qual == NIL); - foreach(l, splan->partitioned_rels) - { - lfirst_int(l) += rtoffset; - } foreach(l, splan->appendplans) { lfirst(l) = set_plan_refs(root, @@ -952,10 +917,6 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) */ set_dummy_tlist_references(plan, rtoffset); Assert(splan->plan.qual == NIL); - foreach(l, splan->partitioned_rels) - { - lfirst_int(l) += rtoffset; - } foreach(l, splan->mergeplans) { lfirst(l) = set_plan_refs(root, diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index c5aaaf5c22..73eca1d4d0 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -3291,10 +3291,8 @@ create_lockrows_path(PlannerInfo *root, RelOptInfo *rel, * 'rel' is the parent relation associated with the result * 'operation' is the operation type * 'canSetTag' is true if we set the command tag/es_processed - * 'nominalRelation' is the parent RT index for use of EXPLAIN - * 'partitioned_rels' is an integer list of RT indexes of non-leaf tables in - * the partition tree, if this is an UPDATE/DELETE to a partitioned table. - * Otherwise NIL. + * 'targetRelation' is the target table's RT index + * 'partitionedTarget' true if 'targetRelation' is a partitioned table * 'partColsUpdated' is true if any partitioning columns are being updated, * either from the target relation or a descendent partitioned table. * 'resultRelations' is an integer list of actual RT indexes of target rel(s) @@ -3309,7 +3307,7 @@ create_lockrows_path(PlannerInfo *root, RelOptInfo *rel, ModifyTablePath * create_modifytable_path(PlannerInfo *root, RelOptInfo *rel, CmdType operation, bool canSetTag, - Index nominalRelation, List *partitioned_rels, + Index targetRelation, bool partitionedTarget, bool partColsUpdated, List *resultRelations, List *subpaths, List *subroots, @@ -3376,8 +3374,8 @@ create_modifytable_path(PlannerInfo *root, RelOptInfo *rel, pathnode->operation = operation; pathnode->canSetTag = canSetTag; - pathnode->nominalRelation = nominalRelation; - pathnode->partitioned_rels = list_copy(partitioned_rels); + pathnode->targetRelation = targetRelation; + pathnode->partitionedTarget = partitionedTarget; pathnode->partColsUpdated = partColsUpdated; pathnode->resultRelations = resultRelations; pathnode->subpaths = subpaths; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index b5804f64ad..d0427c64ba 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -103,7 +103,7 @@ CommandIsReadOnly(PlannedStmt *pstmt) switch (pstmt->commandType) { case CMD_SELECT: - if (pstmt->rowMarks != NIL) + if (pstmt->hasRowMarks) return false; /* SELECT FOR [KEY] UPDATE/SHARE */ else if (pstmt->hasModifyingCTE) return false; /* data-modifying CTE */ @@ -2809,10 +2809,19 @@ CreateCommandTag(Node *parsetree) * will be useful for complaints about read-only * statements */ - if (stmt->rowMarks != NIL) + if (stmt->hasRowMarks) { + List *rowMarks; + + /* Top-level Plan must be LockRows or ModifyTable */ + Assert(IsA(stmt->planTree, LockRows) || + IsA(stmt->planTree, ModifyTable)); + if (IsA(stmt->planTree, LockRows)) + rowMarks = ((LockRows *) stmt->planTree)->rowMarks; + else + rowMarks = ((ModifyTable *) stmt->planTree)->rowMarks; /* not 100% but probably close enough */ - switch (((PlanRowMark *) linitial(stmt->rowMarks))->strength) + switch (((PlanRowMark *) linitial(rowMarks))->strength) { case LCS_FORKEYSHARE: tag = "SELECT FOR KEY SHARE"; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 4c3a78b1a7..4d3de18e07 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -524,8 +524,6 @@ extern void UnregisterExprContextCallback(ExprContext *econtext, ExprContextCallbackFunction function, Datum arg); -extern void ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate); - extern Datum GetAttributeByName(HeapTupleHeader tuple, const char *attname, bool *isNull); extern Datum GetAttributeByNum(HeapTupleHeader tuple, AttrNumber attrno, diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index fbbb278e79..452627dd03 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -51,6 +51,8 @@ typedef struct PlannedStmt bool hasModifyingCTE; /* has insert|update|delete in WITH? */ + bool hasRowMarks; /* has FOR UPDATE/SHARE? */ + bool canSetTag; /* do I set the command result tag? */ bool transientPlan; /* redo plan when TransactionXmin changes? */ @@ -69,15 +71,8 @@ typedef struct PlannedStmt List *resultRelations; /* integer list of RT indexes, or NIL */ /* - * rtable indexes of non-leaf target relations for UPDATE/DELETE on all - * the partitioned tables mentioned in the query. - */ - List *nonleafResultRelations; - - /* - * rtable indexes of root target relations for UPDATE/DELETE; this list - * maintains a subset of the RT indexes in nonleafResultRelations, - * indicating the roots of the respective partition hierarchies. + * rtable indexes of root partitioned tables that are UPDATE/DELETE + * targets */ List *rootResultRelations; @@ -86,8 +81,6 @@ typedef struct PlannedStmt Bitmapset *rewindPlanIDs; /* indices of subplans that require REWIND */ - List *rowMarks; /* a list of PlanRowMark's */ - List *relationOids; /* OIDs of relations the plan depends on */ List *invalItems; /* other dependencies, as PlanInvalItems */ @@ -219,9 +212,10 @@ typedef struct ModifyTable Plan plan; CmdType operation; /* INSERT, UPDATE, or DELETE */ bool canSetTag; /* do we set the command tag/es_processed? */ - Index nominalRelation; /* Parent RT index for use of EXPLAIN */ - /* RT indexes of non-leaf tables in a partition tree */ - List *partitioned_rels; + Index targetRelation; /* RT index of the target table specified in + * the command or first child for inheritance + * tables */ + bool partitionedTarget; /* is the target table partitioned? */ bool partColsUpdated; /* some part key in hierarchy updated */ List *resultRelations; /* integer list of RT indexes */ int resultRelIndex; /* index of first resultRel in plan's list */ @@ -259,9 +253,6 @@ typedef struct Append */ int first_partial_plan; - /* RT indexes of non-leaf tables in a partition tree */ - List *partitioned_rels; - /* Info for run-time subplan pruning; NULL if we're not doing that */ struct PartitionPruneInfo *part_prune_info; } Append; @@ -274,8 +265,6 @@ typedef struct Append typedef struct MergeAppend { Plan plan; - /* RT indexes of non-leaf tables in a partition tree */ - List *partitioned_rels; List *mergeplans; /* remaining fields are just like the sort-key info in struct Sort */ int numCols; /* number of sort-key columns */ @@ -927,8 +916,7 @@ typedef struct SetOp /* ---------------- * lock-rows node * - * rowMarks identifies the rels to be locked by this node; it should be - * a subset of the rowMarks listed in the top-level PlannedStmt. + * rowMarks identifies the rels to be locked by this node. * epqParam is a Param that all scan nodes below this one must depend on. * It is used to force re-evaluation of the plan during EvalPlanQual. * ---------------- diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index adb4265047..bcc2ba6db5 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -117,11 +117,8 @@ typedef struct PlannerGlobal List *finalrtable; /* "flat" rangetable for executor */ - List *finalrowmarks; /* "flat" list of PlanRowMarks */ - List *resultRelations; /* "flat" list of integer RT indexes */ - List *nonleafResultRelations; /* "flat" list of integer RT indexes */ List *rootResultRelations; /* "flat" list of integer RT indexes */ List *relationOids; /* OIDs of relations the plan depends on */ @@ -1716,9 +1713,10 @@ typedef struct ModifyTablePath Path path; CmdType operation; /* INSERT, UPDATE, or DELETE */ bool canSetTag; /* do we set the command tag/es_processed? */ - Index nominalRelation; /* Parent RT index for use of EXPLAIN */ - /* RT indexes of non-leaf tables in a partition tree */ - List *partitioned_rels; + Index targetRelation; /* RT index of the target table specified in + * the command or first child for inheritance + * tables */ + bool partitionedTarget; /* is the target table partitioned? */ bool partColsUpdated; /* some part key in hierarchy updated */ List *resultRelations; /* integer list of RT indexes */ List *subpaths; /* Path(s) producing source data */ diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index 7c5ff22650..a45c2407d0 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -238,7 +238,7 @@ extern LockRowsPath *create_lockrows_path(PlannerInfo *root, RelOptInfo *rel, extern ModifyTablePath *create_modifytable_path(PlannerInfo *root, RelOptInfo *rel, CmdType operation, bool canSetTag, - Index nominalRelation, List *partitioned_rels, + Index targetRelation, bool partitionedTarget, bool partColsUpdated, List *resultRelations, List *subpaths, List *subroots, -- 2.11.0
From f48e6b91cebe393da71d6412b07f00cc65758bc4 Mon Sep 17 00:00:00 2001 From: "dgrow...@gmail.com" <dgrow...@gmail.com> Date: Thu, 27 Sep 2018 13:31:11 +1200 Subject: [PATCH v12 5/5] Prune PlanRowMark of relations that are pruned from a plan --- src/backend/optimizer/plan/planner.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 6a5d2f9d99..83e1021d12 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -248,6 +248,7 @@ static bool group_by_has_partkey(RelOptInfo *input_rel, List *targetList, List *groupClause); static int common_prefix_cmp(const void *a, const void *b); +static List *get_nondummy_rowmarks(PlannerInfo *root); /***************************************************************************** @@ -1595,7 +1596,7 @@ inheritance_planner(PlannerInfo *root) if (parse->rowMarks) rowMarks = NIL; else - rowMarks = root->rowMarks; + rowMarks = get_nondummy_rowmarks(root); /* Create Path representing a ModifyTable to do the UPDATE/DELETE work */ add_path(final_rel, (Path *) @@ -2124,11 +2125,9 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, * is what goes into the LockRows node.) */ if (parse->rowMarks) - { path = (Path *) create_lockrows_path(root, final_rel, path, - root->rowMarks, + get_nondummy_rowmarks(root), SS_assign_special_param(root)); - } /* * If there is a LIMIT/OFFSET clause, add the LIMIT node. @@ -2173,7 +2172,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, if (parse->rowMarks) rowMarks = NIL; else - rowMarks = root->rowMarks; + rowMarks = get_nondummy_rowmarks(root); path = (Path *) create_modifytable_path(root, final_rel, @@ -7221,3 +7220,25 @@ group_by_has_partkey(RelOptInfo *input_rel, return true; } + +/* + * get_nondummy_rowmarks + * Return a list of PlanRowMark's that reference non-dummy relations + */ +static List * +get_nondummy_rowmarks(PlannerInfo *root) +{ + List *rowMarks = NIL; + ListCell *lc; + + foreach(lc, root->rowMarks) + { + PlanRowMark *rc = lfirst(lc); + + if (root->simple_rel_array[rc->rti] != NULL && + !IS_DUMMY_REL(root->simple_rel_array[rc->rti])) + rowMarks = lappend(rowMarks, rc); + } + + return rowMarks; +} -- 2.11.0