Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f234365

Browse filesBrowse files
tglsfdctglsfdc
committed
Remove more redundant relation locking during executor startup.
We already have appropriate locks on every relation listed in the query's rangetable before we reach the executor. Take the next step in exploiting that knowledge by removing code that worries about taking locks on non-leaf result relations in a partitioned table. In particular, get rid of ExecLockNonLeafAppendTables and a stanza in InitPlan that asserts we already have locks on certain such tables. In passing, clean up some now-obsolete comments in InitPlan. Amit Langote, reviewed by David Rowley and Jesper Pedersen, and whacked around a bit more by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
1 parent 0209f02 commit f234365
Copy full SHA for f234365

File tree

Expand file treeCollapse file tree

5 files changed

+14
-116
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+14
-116
lines changed

‎src/backend/executor/execMain.c

Copy file name to clipboardExpand all lines: src/backend/executor/execMain.c
+14-42Lines changed: 14 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -828,10 +828,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
828828
estate->es_plannedstmt = plannedstmt;
829829

830830
/*
831-
* initialize result relation stuff, and open/lock the result rels.
832-
*
833-
* We must do this before initializing the plan tree, else we might try to
834-
* do a lock upgrade if a result rel is also a source rel.
831+
* Initialize ResultRelInfo data structures, and open the result rels.
835832
*/
836833
if (plannedstmt->resultRelations)
837834
{
@@ -859,25 +856,19 @@ InitPlan(QueryDesc *queryDesc, int eflags)
859856
}
860857
estate->es_result_relations = resultRelInfos;
861858
estate->es_num_result_relations = numResultRelations;
859+
862860
/* es_result_relation_info is NULL except when within ModifyTable */
863861
estate->es_result_relation_info = NULL;
864862

865863
/*
866-
* In the partitioned result relation case, lock the non-leaf result
867-
* relations too. A subset of these are the roots of respective
868-
* partitioned tables, for which we also allocate ResultRelInfos.
864+
* In the partitioned result relation case, also build ResultRelInfos
865+
* for all the partitioned table roots, because we will need them to
866+
* fire statement-level triggers, if any.
869867
*/
870-
estate->es_root_result_relations = NULL;
871-
estate->es_num_root_result_relations = 0;
872-
if (plannedstmt->nonleafResultRelations)
868+
if (plannedstmt->rootResultRelations)
873869
{
874870
int num_roots = list_length(plannedstmt->rootResultRelations);
875871

876-
/*
877-
* Firstly, build ResultRelInfos for all the partitioned table
878-
* roots, because we will need them to fire the statement-level
879-
* triggers, if any.
880-
*/
881872
resultRelInfos = (ResultRelInfo *)
882873
palloc(num_roots * sizeof(ResultRelInfo));
883874
resultRelInfo = resultRelInfos;
@@ -898,26 +889,11 @@ InitPlan(QueryDesc *queryDesc, int eflags)
898889

899890
estate->es_root_result_relations = resultRelInfos;
900891
estate->es_num_root_result_relations = num_roots;
901-
902-
/* Simply check the rest of them are locked. */
903-
#ifdef USE_ASSERT_CHECKING
904-
foreach(l, plannedstmt->nonleafResultRelations)
905-
{
906-
Index resultRelIndex = lfirst_int(l);
907-
908-
/* We locked the roots above. */
909-
if (!list_member_int(plannedstmt->rootResultRelations,
910-
resultRelIndex))
911-
{
912-
Relation resultRelDesc;
913-
Oid reloid = exec_rt_fetch(resultRelIndex, estate)->relid;
914-
915-
resultRelDesc = heap_open(reloid, NoLock);
916-
Assert(CheckRelationLockedByMe(resultRelDesc, RowExclusiveLock, true));
917-
heap_close(resultRelDesc, NoLock);
918-
}
919-
}
920-
#endif
892+
}
893+
else
894+
{
895+
estate->es_root_result_relations = NULL;
896+
estate->es_num_root_result_relations = 0;
921897
}
922898
}
923899
else
@@ -933,13 +909,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
933909
}
934910

935911
/*
936-
* Similarly, we have to lock relations selected FOR [KEY] UPDATE/SHARE
937-
* before we initialize the plan tree, else we'd be risking lock upgrades.
938-
* While we are at it, build the ExecRowMark list. Any partitioned child
939-
* tables are ignored here (because isParent=true) and will be locked by
940-
* the first Append or MergeAppend node that references them. (Note that
941-
* the RowMarks corresponding to partitioned child tables are present in
942-
* the same list as the rest, i.e., plannedstmt->rowMarks.)
912+
* Next, build the ExecRowMark list from the PlanRowMark(s), if any.
943913
*/
944914
estate->es_rowMarks = NIL;
945915
foreach(l, plannedstmt->rowMarks)
@@ -956,6 +926,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
956926
/* get relation's OID (will produce InvalidOid if subquery) */
957927
relid = exec_rt_fetch(rc->rti, estate)->relid;
958928

929+
/* open relation, if we need to access it for this mark type */
959930
switch (rc->markType)
960931
{
961932
case ROW_MARK_EXCLUSIVE:
@@ -991,6 +962,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
991962
erm->ermActive = false;
992963
ItemPointerSetInvalid(&(erm->curCtid));
993964
erm->ermExtra = NULL;
965+
994966
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
995967
}
996968

‎src/backend/executor/execUtils.c

Copy file name to clipboardExpand all lines: src/backend/executor/execUtils.c
-60Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -899,66 +899,6 @@ ShutdownExprContext(ExprContext *econtext, bool isCommit)
899899
MemoryContextSwitchTo(oldcontext);
900900
}
901901

902-
/*
903-
* ExecLockNonLeafAppendTables
904-
*
905-
* Locks, if necessary, the tables indicated by the RT indexes contained in
906-
* the partitioned_rels list. These are the non-leaf tables in the partition
907-
* tree controlled by a given Append or MergeAppend node.
908-
*/
909-
void
910-
ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate)
911-
{
912-
PlannedStmt *stmt = estate->es_plannedstmt;
913-
ListCell *lc;
914-
915-
foreach(lc, partitioned_rels)
916-
{
917-
ListCell *l;
918-
Index rti = lfirst_int(lc);
919-
bool is_result_rel = false;
920-
Oid relid = exec_rt_fetch(rti, estate)->relid;
921-
922-
/* If this is a result relation, already locked in InitPlan */
923-
foreach(l, stmt->nonleafResultRelations)
924-
{
925-
if (rti == lfirst_int(l))
926-
{
927-
is_result_rel = true;
928-
break;
929-
}
930-
}
931-
932-
/*
933-
* Not a result relation; check if there is a RowMark that requires
934-
* taking a RowShareLock on this rel.
935-
*/
936-
if (!is_result_rel)
937-
{
938-
PlanRowMark *rc = NULL;
939-
LOCKMODE lockmode;
940-
941-
foreach(l, stmt->rowMarks)
942-
{
943-
if (((PlanRowMark *) lfirst(l))->rti == rti)
944-
{
945-
rc = lfirst(l);
946-
break;
947-
}
948-
}
949-
950-
if (rc && RowMarkRequiresRowShareLock(rc->markType))
951-
lockmode = RowShareLock;
952-
else
953-
lockmode = AccessShareLock;
954-
955-
Assert(lockmode == exec_rt_fetch(rti, estate)->rellockmode);
956-
957-
LockRelationOid(relid, lockmode);
958-
}
959-
}
960-
}
961-
962902
/*
963903
* GetAttributeByName
964904
* GetAttributeByNum

‎src/backend/executor/nodeAppend.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeAppend.c
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,6 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
112112
/* check for unsupported flags */
113113
Assert(!(eflags & EXEC_FLAG_MARK));
114114

115-
/*
116-
* Lock the non-leaf tables in the partition tree controlled by this node.
117-
* It's a no-op for non-partitioned parent tables.
118-
*/
119-
ExecLockNonLeafAppendTables(node->partitioned_rels, estate);
120-
121115
/*
122116
* create new AppendState for our append node
123117
*/

‎src/backend/executor/nodeMergeAppend.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeMergeAppend.c
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,6 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
7575
/* check for unsupported flags */
7676
Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
7777

78-
/*
79-
* Lock the non-leaf tables in the partition tree controlled by this node.
80-
* It's a no-op for non-partitioned parent tables.
81-
*/
82-
ExecLockNonLeafAppendTables(node->partitioned_rels, estate);
83-
8478
/*
8579
* create new MergeAppendState for our node
8680
*/

‎src/include/executor/executor.h

Copy file name to clipboardExpand all lines: src/include/executor/executor.h
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,8 +534,6 @@ extern void UnregisterExprContextCallback(ExprContext *econtext,
534534
ExprContextCallbackFunction function,
535535
Datum arg);
536536

537-
extern void ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate);
538-
539537
extern Datum GetAttributeByName(HeapTupleHeader tuple, const char *attname,
540538
bool *isNull);
541539
extern Datum GetAttributeByNum(HeapTupleHeader tuple, AttrNumber attrno,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.