Skip to content

Commit 9ddef36

Browse files
tglsfdctglsfdc
committed
Centralize executor's opening/closing of Relations for rangetable entries.
Create an array estate->es_relations[] paralleling the es_range_table, and store references to Relations (relcache entries) there, so that any given RT entry is opened and closed just once per executor run. Scan nodes typically still call ExecOpenScanRelation, but ExecCloseScanRelation is no more; relation closing is now done centrally in ExecEndPlan. This is slightly more complex than one would expect because of the interactions with relcache references held in ResultRelInfo nodes. The general convention is now that ResultRelInfo->ri_RelationDesc does not represent a separate relcache reference and so does not need to be explicitly closed; but there is an exception for ResultRelInfos in the es_trig_target_relations list, which are manufactured by ExecGetTriggerResultRel and have to be cleaned up by ExecCleanUpTriggerState. (That much was true all along, but these ResultRelInfos are now more different from others than they used to be.) To allow the partition pruning logic to make use of es_relations[] rather than having its own relcache references, adjust PartitionedRelPruneInfo to store an RT index rather than a relation OID. Amit Langote, reviewed by David Rowley and Jesper Pedersen, some mods by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
1 parent fb9e93a commit 9ddef36

27 files changed

+144
-181
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2546,10 +2546,6 @@ postgresEndDirectModify(ForeignScanState *node)
25462546
ReleaseConnection(dmstate->conn);
25472547
dmstate->conn = NULL;
25482548

2549-
/* close the target relation. */
2550-
if (dmstate->resultRel)
2551-
ExecCloseScanRelation(dmstate->resultRel);
2552-
25532549
/* MemoryContext will be deleted automatically. */
25542550
}
25552551

‎src/backend/catalog/indexing.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ CatalogOpenIndexes(Relation heapRel)
4242
ResultRelInfo *resultRelInfo;
4343

4444
resultRelInfo = makeNode(ResultRelInfo);
45-
resultRelInfo->ri_RangeTableIndex = 1; /* dummy */
45+
resultRelInfo->ri_RangeTableIndex = 0; /* dummy */
4646
resultRelInfo->ri_RelationDesc = heapRel;
4747
resultRelInfo->ri_TrigDesc = NULL; /* we don't fire triggers */
4848

‎src/backend/commands/copy.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2471,7 +2471,7 @@ CopyFrom(CopyState cstate)
24712471
resultRelInfo = makeNode(ResultRelInfo);
24722472
InitResultRelInfo(resultRelInfo,
24732473
cstate->rel,
2474-
1, /* dummy rangetable index */
2474+
1, /* must match rel's position in range_table */
24752475
NULL,
24762476
0);
24772477
target_resultRelInfo = resultRelInfo;
@@ -2485,6 +2485,8 @@ CopyFrom(CopyState cstate)
24852485
estate->es_num_result_relations = 1;
24862486
estate->es_result_relation_info = resultRelInfo;
24872487
estate->es_range_table = cstate->range_table;
2488+
estate->es_relations = (Relation *) palloc0(list_length(cstate->range_table) *
2489+
sizeof(Relation));
24882490

24892491
/* Set up a tuple slot too */
24902492
myslot = ExecInitExtraTupleSlot(estate, tupDesc);

‎src/backend/executor/execMain.c

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,15 @@ InitPlan(QueryDesc *queryDesc, int eflags)
824824
* initialize the node's execution state
825825
*/
826826
estate->es_range_table = rangeTable;
827+
828+
/*
829+
* Allocate an array to store an open Relation corresponding to each
830+
* rangeTable item, and initialize entries to NULL. Relations are opened
831+
* and stored here as needed.
832+
*/
833+
estate->es_relations = (Relation *) palloc0(list_length(rangeTable) *
834+
sizeof(Relation));
835+
827836
estate->es_plannedstmt = plannedstmt;
828837

829838
/*
@@ -845,13 +854,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
845854
foreach(l, resultRelations)
846855
{
847856
Index resultRelationIndex = lfirst_int(l);
848-
Oid resultRelationOid;
849857
Relation resultRelation;
850858

851-
resultRelationOid = getrelid(resultRelationIndex, rangeTable);
852-
resultRelation = heap_open(resultRelationOid, NoLock);
853-
Assert(CheckRelationLockedByMe(resultRelation, RowExclusiveLock, true));
854-
859+
resultRelation = ExecGetRangeTableRelation(estate,
860+
resultRelationIndex);
855861
InitResultRelInfo(resultRelInfo,
856862
resultRelation,
857863
resultRelationIndex,
@@ -886,15 +892,13 @@ InitPlan(QueryDesc *queryDesc, int eflags)
886892
foreach(l, plannedstmt->rootResultRelations)
887893
{
888894
Index resultRelIndex = lfirst_int(l);
889-
Oid resultRelOid;
890895
Relation resultRelDesc;
891896

892-
resultRelOid = getrelid(resultRelIndex, rangeTable);
893-
resultRelDesc = heap_open(resultRelOid, NoLock);
894-
Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true));
897+
resultRelDesc = ExecGetRangeTableRelation(estate,
898+
resultRelIndex);
895899
InitResultRelInfo(resultRelInfo,
896900
resultRelDesc,
897-
lfirst_int(l),
901+
resultRelIndex,
898902
NULL,
899903
estate->es_instrument);
900904
resultRelInfo++;
@@ -967,10 +971,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
967971
case ROW_MARK_SHARE:
968972
case ROW_MARK_KEYSHARE:
969973
case ROW_MARK_REFERENCE:
970-
relation = heap_open(relid, NoLock);
971-
Assert(CheckRelationLockedByMe(relation,
972-
rt_fetch(rc->rti, rangeTable)->rellockmode,
973-
true));
974+
relation = ExecGetRangeTableRelation(estate, rc->rti);
974975
break;
975976
case ROW_MARK_COPY:
976977
/* no physical table access is required */
@@ -1483,8 +1484,19 @@ ExecCleanUpTriggerState(EState *estate)
14831484
{
14841485
ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l);
14851486

1486-
/* Close indices and then the relation itself */
1487-
ExecCloseIndices(resultRelInfo);
1487+
/*
1488+
* Assert this is a "dummy" ResultRelInfo, see above. Otherwise we
1489+
* might be issuing a duplicate close against a Relation opened by
1490+
* ExecGetRangeTableRelation.
1491+
*/
1492+
Assert(resultRelInfo->ri_RangeTableIndex == 0);
1493+
1494+
/*
1495+
* Since ExecGetTriggerResultRel doesn't call ExecOpenIndices for
1496+
* these rels, we needn't call ExecCloseIndices either.
1497+
*/
1498+
Assert(resultRelInfo->ri_NumIndices == 0);
1499+
14881500
heap_close(resultRelInfo->ri_RelationDesc, NoLock);
14891501
}
14901502
}
@@ -1607,6 +1619,7 @@ static void
16071619
ExecEndPlan(PlanState *planstate, EState *estate)
16081620
{
16091621
ResultRelInfo *resultRelInfo;
1622+
int num_relations;
16101623
int i;
16111624
ListCell *l;
16121625

@@ -1634,39 +1647,29 @@ ExecEndPlan(PlanState *planstate, EState *estate)
16341647
ExecResetTupleTable(estate->es_tupleTable, false);
16351648

16361649
/*
1637-
* close the result relation(s) if any, but hold locks until xact commit.
1650+
* close indexes of result relation(s) if any. (Rels themselves get
1651+
* closed next.)
16381652
*/
16391653
resultRelInfo = estate->es_result_relations;
16401654
for (i = estate->es_num_result_relations; i > 0; i--)
16411655
{
1642-
/* Close indices and then the relation itself */
16431656
ExecCloseIndices(resultRelInfo);
1644-
heap_close(resultRelInfo->ri_RelationDesc, NoLock);
16451657
resultRelInfo++;
16461658
}
16471659

1648-
/* Close the root target relation(s). */
1649-
resultRelInfo = estate->es_root_result_relations;
1650-
for (i = estate->es_num_root_result_relations; i > 0; i--)
1660+
/*
1661+
* close whatever rangetable Relations have been opened. We did not
1662+
* acquire locks in ExecGetRangeTableRelation, so don't release 'em here.
1663+
*/
1664+
num_relations = list_length(estate->es_range_table);
1665+
for (i = 0; i < num_relations; i++)
16511666
{
1652-
heap_close(resultRelInfo->ri_RelationDesc, NoLock);
1653-
resultRelInfo++;
1667+
if (estate->es_relations[i])
1668+
heap_close(estate->es_relations[i], NoLock);
16541669
}
16551670

16561671
/* likewise close any trigger target relations */
16571672
ExecCleanUpTriggerState(estate);
1658-
1659-
/*
1660-
* close any relations selected FOR [KEY] UPDATE/SHARE, again keeping
1661-
* locks
1662-
*/
1663-
foreach(l, estate->es_rowMarks)
1664-
{
1665-
ExecRowMark *erm = (ExecRowMark *) lfirst(l);
1666-
1667-
if (erm->relation)
1668-
heap_close(erm->relation, NoLock);
1669-
}
16701673
}
16711674

16721675
/* ----------------------------------------------------------------
@@ -3161,6 +3164,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
31613164
estate->es_snapshot = parentestate->es_snapshot;
31623165
estate->es_crosscheck_snapshot = parentestate->es_crosscheck_snapshot;
31633166
estate->es_range_table = parentestate->es_range_table;
3167+
estate->es_relations = parentestate->es_relations;
31643168
estate->es_plannedstmt = parentestate->es_plannedstmt;
31653169
estate->es_junkFilter = parentestate->es_junkFilter;
31663170
estate->es_output_cid = parentestate->es_output_cid;

‎src/backend/executor/execPartition.c

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd,
219219
ExprContext *ecxt = GetPerTupleExprContext(estate);
220220
TupleTableSlot *ecxt_scantuple_old = ecxt->ecxt_scantuple;
221221
TupleTableSlot *myslot = NULL;
222-
MemoryContext oldcxt;
222+
MemoryContext oldcxt;
223223

224224
/* use per-tuple context here to avoid leaking memory */
225225
oldcxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
@@ -1389,9 +1389,6 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
13891389
* functions. Details stored include how to map the partition index
13901390
* returned by the partition pruning code into subplan indexes.
13911391
*
1392-
* ExecDestroyPartitionPruneState:
1393-
* Deletes a PartitionPruneState. Must be called during executor shutdown.
1394-
*
13951392
* ExecFindInitialMatchingSubPlans:
13961393
* Returns indexes of matching subplans. Partition pruning is attempted
13971394
* without any evaluation of expressions containing PARAM_EXEC Params.
@@ -1433,6 +1430,7 @@ PartitionPruneState *
14331430
ExecCreatePartitionPruneState(PlanState *planstate,
14341431
PartitionPruneInfo *partitionpruneinfo)
14351432
{
1433+
EState *estate = planstate->state;
14361434
PartitionPruneState *prunestate;
14371435
int n_part_hierarchies;
14381436
ListCell *lc;
@@ -1487,6 +1485,7 @@ ExecCreatePartitionPruneState(PlanState *planstate,
14871485
PartitionedRelPruneInfo *pinfo = lfirst_node(PartitionedRelPruneInfo, lc2);
14881486
PartitionedRelPruningData *pprune = &prunedata->partrelprunedata[j];
14891487
PartitionPruneContext *context = &pprune->context;
1488+
Relation partrel;
14901489
PartitionDesc partdesc;
14911490
PartitionKey partkey;
14921491
int partnatts;
@@ -1509,16 +1508,15 @@ ExecCreatePartitionPruneState(PlanState *planstate,
15091508
pprune->present_parts = bms_copy(pinfo->present_parts);
15101509

15111510
/*
1512-
* We need to hold a pin on the partitioned table's relcache entry
1513-
* so that we can rely on its copies of the table's partition key
1514-
* and partition descriptor. We need not get a lock though; one
1515-
* should have been acquired already by InitPlan or
1516-
* ExecLockNonLeafAppendTables.
1511+
* We can rely on the copies of the partitioned table's partition
1512+
* key and partition descriptor appearing in its relcache entry,
1513+
* because that entry will be held open and locked for the
1514+
* duration of this executor run.
15171515
*/
1518-
context->partrel = relation_open(pinfo->reloid, NoLock);
1516+
partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
1517+
partkey = RelationGetPartitionKey(partrel);
1518+
partdesc = RelationGetPartitionDesc(partrel);
15191519

1520-
partkey = RelationGetPartitionKey(context->partrel);
1521-
partdesc = RelationGetPartitionDesc(context->partrel);
15221520
n_steps = list_length(pinfo->pruning_steps);
15231521

15241522
context->strategy = partkey->strategy;
@@ -1595,30 +1593,6 @@ ExecCreatePartitionPruneState(PlanState *planstate,
15951593
return prunestate;
15961594
}
15971595

1598-
/*
1599-
* ExecDestroyPartitionPruneState
1600-
* Release resources at plan shutdown.
1601-
*
1602-
* We don't bother to free any memory here, since the whole executor context
1603-
* will be going away shortly. We do need to release our relcache pins.
1604-
*/
1605-
void
1606-
ExecDestroyPartitionPruneState(PartitionPruneState *prunestate)
1607-
{
1608-
PartitionPruningData **partprunedata = prunestate->partprunedata;
1609-
int i;
1610-
1611-
for (i = 0; i < prunestate->num_partprunedata; i++)
1612-
{
1613-
PartitionPruningData *prunedata = partprunedata[i];
1614-
PartitionedRelPruningData *pprune = prunedata->partrelprunedata;
1615-
int j;
1616-
1617-
for (j = 0; j < prunedata->num_partrelprunedata; j++)
1618-
relation_close(pprune[j].context.partrel, NoLock);
1619-
}
1620-
}
1621-
16221596
/*
16231597
* ExecFindInitialMatchingSubPlans
16241598
* Identify the set of subplans that cannot be eliminated by initial

‎src/backend/executor/execUtils.c

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
* etc
2626
*
2727
* ExecOpenScanRelation Common code for scan node init routines.
28-
* ExecCloseScanRelation
28+
*
29+
* ExecGetRangeTableRelation Fetch Relation for a rangetable entry.
2930
*
3031
* executor_errposition Report syntactic position of an error.
3132
*
@@ -107,6 +108,7 @@ CreateExecutorState(void)
107108
estate->es_snapshot = InvalidSnapshot; /* caller must initialize this */
108109
estate->es_crosscheck_snapshot = InvalidSnapshot; /* no crosscheck */
109110
estate->es_range_table = NIL;
111+
estate->es_relations = NULL;
110112
estate->es_plannedstmt = NULL;
111113

112114
estate->es_junkFilter = NULL;
@@ -648,15 +650,9 @@ Relation
648650
ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
649651
{
650652
Relation rel;
651-
Oid reloid;
652653

653-
/* Open the relation and verify lock was obtained upstream */
654-
reloid = getrelid(scanrelid, estate->es_range_table);
655-
rel = heap_open(reloid, NoLock);
656-
Assert(IsParallelWorker() ||
657-
CheckRelationLockedByMe(rel,
658-
rt_fetch(scanrelid, estate->es_range_table)->rellockmode,
659-
true));
654+
/* Open the relation. */
655+
rel = ExecGetRangeTableRelation(estate, scanrelid);
660656

661657
/*
662658
* Complain if we're attempting a scan of an unscannable relation, except
@@ -674,24 +670,40 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
674670
return rel;
675671
}
676672

677-
/* ----------------------------------------------------------------
678-
* ExecCloseScanRelation
679-
*
680-
* Close the heap relation scanned by a base-level scan plan node.
681-
* This should be called during the node's ExecEnd routine.
682-
*
683-
* Currently, we do not release the lock acquired by ExecOpenScanRelation.
684-
* This lock should be held till end of transaction. (There is a faction
685-
* that considers this too much locking, however.)
673+
/*
674+
* ExecGetRangeTableRelation
675+
* Open the Relation for a range table entry, if not already done
686676
*
687-
* If we did want to release the lock, we'd have to repeat the logic in
688-
* ExecOpenScanRelation in order to figure out what to release.
689-
* ----------------------------------------------------------------
677+
* The Relations will be closed again in ExecEndPlan().
690678
*/
691-
void
692-
ExecCloseScanRelation(Relation scanrel)
679+
Relation
680+
ExecGetRangeTableRelation(EState *estate, Index rti)
693681
{
694-
heap_close(scanrel, NoLock);
682+
Relation rel;
683+
684+
Assert(rti > 0 && rti <= list_length(estate->es_range_table));
685+
686+
rel = estate->es_relations[rti - 1];
687+
if (rel == NULL)
688+
{
689+
/* First time through, so open the relation */
690+
RangeTblEntry *rte = rt_fetch(rti, estate->es_range_table);
691+
692+
Assert(rte->rtekind == RTE_RELATION);
693+
694+
rel = estate->es_relations[rti - 1] = heap_open(rte->relid, NoLock);
695+
696+
/*
697+
* Verify that appropriate lock was obtained before execution.
698+
*
699+
* In the case of parallel query, only the leader would've obtained
700+
* the lock (that needs to be fixed, though).
701+
*/
702+
Assert(IsParallelWorker() ||
703+
CheckRelationLockedByMe(rel, rte->rellockmode, false));
704+
}
705+
706+
return rel;
695707
}
696708

697709
/*

‎src/backend/executor/nodeAppend.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -335,12 +335,6 @@ ExecEndAppend(AppendState *node)
335335
*/
336336
for (i = 0; i < nplans; i++)
337337
ExecEndNode(appendplans[i]);
338-
339-
/*
340-
* release any resources associated with run-time pruning
341-
*/
342-
if (node->as_prune_state)
343-
ExecDestroyPartitionPruneState(node->as_prune_state);
344338
}
345339

346340
void

0 commit comments

Comments
 (0)