From 4255d72da27982934b27da9ae5268ba1d553c2d8 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 23 Aug 2018 17:30:18 +0900 Subject: [PATCH v3 3/4] Only lock partitions that will be scanned by a query --- src/backend/optimizer/prep/preptlist.c | 127 ++++++++++++++++++--------------- src/backend/optimizer/prep/prepunion.c | 15 ++-- src/backend/optimizer/util/relnode.c | 36 +++++++--- src/include/optimizer/prep.h | 6 +- 4 files changed, 111 insertions(+), 73 deletions(-) diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index 8603feef2b..3f93e5933e 100644 --- a/src/backend/optimizer/prep/preptlist.c +++ b/src/backend/optimizer/prep/preptlist.c @@ -76,7 +76,6 @@ preprocess_targetlist(PlannerInfo *root) RangeTblEntry *target_rte = NULL; Relation target_relation = NULL; List *tlist; - ListCell *lc; /* * If there is a result relation, open it so we can look for missing @@ -118,12 +117,78 @@ preprocess_targetlist(PlannerInfo *root) tlist = expand_targetlist(tlist, command_type, result_relation, target_relation); + + tlist = add_rowmark_columns(root, tlist, root->rowMarks); + /* - * Add necessary junk columns for rowmarked rels. These values are needed - * for locking of rels selected FOR UPDATE/SHARE, and to do EvalPlanQual - * rechecking. See comments for PlanRowMark in plannodes.h. + * If the query has a RETURNING list, add resjunk entries for any Vars + * used in RETURNING that belong to other relations. We need to do this + * to make these Vars available for the RETURNING calculation. Vars that + * belong to the result rel don't need to be added, because they will be + * made to refer to the actual heap tuple. */ - foreach(lc, root->rowMarks) + if (parse->returningList && list_length(parse->rtable) > 1) + { + List *vars; + ListCell *l; + + vars = pull_var_clause((Node *) parse->returningList, + PVC_RECURSE_AGGREGATES | + PVC_RECURSE_WINDOWFUNCS | + PVC_INCLUDE_PLACEHOLDERS); + foreach(l, vars) + { + Var *var = (Var *) lfirst(l); + TargetEntry *tle; + + if (IsA(var, Var) && + var->varno == result_relation) + continue; /* don't need it */ + + if (tlist_member((Expr *) var, tlist)) + continue; /* already got it */ + + tle = makeTargetEntry((Expr *) var, + list_length(tlist) + 1, + NULL, + true); + + tlist = lappend(tlist, tle); + } + list_free(vars); + } + + /* + * If there's an ON CONFLICT UPDATE clause, preprocess its targetlist too + * while we have the relation open. + */ + if (parse->onConflict) + parse->onConflict->onConflictSet = + expand_targetlist(parse->onConflict->onConflictSet, + CMD_UPDATE, + result_relation, + target_relation); + + if (target_relation) + heap_close(target_relation, NoLock); + + return tlist; +} + +/* + * add_rowmark_columns + * + * Add necessary junk columns for rowmarked rels. These values are needed + * for locking of rels selected FOR UPDATE/SHARE, and to do EvalPlanQual + * rechecking. See comments for PlanRowMark in plannodes.h. + */ +List * +add_rowmark_columns(PlannerInfo *root, List *tlist, List *rowMarks) +{ + List *range_table = root->parse->rtable; + ListCell *lc; + + foreach(lc, rowMarks) { PlanRowMark *rc = (PlanRowMark *) lfirst(lc); Var *var; @@ -183,58 +248,6 @@ preprocess_targetlist(PlannerInfo *root) } } - /* - * If the query has a RETURNING list, add resjunk entries for any Vars - * used in RETURNING that belong to other relations. We need to do this - * to make these Vars available for the RETURNING calculation. Vars that - * belong to the result rel don't need to be added, because they will be - * made to refer to the actual heap tuple. - */ - if (parse->returningList && list_length(parse->rtable) > 1) - { - List *vars; - ListCell *l; - - vars = pull_var_clause((Node *) parse->returningList, - PVC_RECURSE_AGGREGATES | - PVC_RECURSE_WINDOWFUNCS | - PVC_INCLUDE_PLACEHOLDERS); - foreach(l, vars) - { - Var *var = (Var *) lfirst(l); - TargetEntry *tle; - - if (IsA(var, Var) && - var->varno == result_relation) - continue; /* don't need it */ - - if (tlist_member((Expr *) var, tlist)) - continue; /* already got it */ - - tle = makeTargetEntry((Expr *) var, - list_length(tlist) + 1, - NULL, - true); - - tlist = lappend(tlist, tle); - } - list_free(vars); - } - - /* - * If there's an ON CONFLICT UPDATE clause, preprocess its targetlist too - * while we have the relation open. - */ - if (parse->onConflict) - parse->onConflict->onConflictSet = - expand_targetlist(parse->onConflict->onConflictSet, - CMD_UPDATE, - result_relation, - target_relation); - - if (target_relation) - heap_close(target_relation, NoLock); - return tlist; } diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 279f686fb0..4d9583e63b 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1555,14 +1555,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) lockmode = AccessShareLock; /* Scan for all members of inheritance set, acquire needed locks */ - inhOIDs = find_all_inheritors(parentOID, lockmode, NULL); + if (rte->relkind != RELKIND_PARTITIONED_TABLE) + inhOIDs = find_all_inheritors(parentOID, lockmode, NULL); /* * Check that there's at least one descendant, else treat as no-child * case. This could happen despite above has_subclass() check, if table * once had a child but no longer does. */ - if (list_length(inhOIDs) < 2) + if (rte->relkind != RELKIND_PARTITIONED_TABLE && list_length(inhOIDs) < 2) { /* Clear flag before returning */ rte->inh = false; @@ -1579,10 +1580,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) /* Partitioned tables are expanded elsewhere. */ if (rte->relkind == RELKIND_PARTITIONED_TABLE) - { - list_free(inhOIDs); return; - } /* * Must open the parent relation to examine its tupdesc. We need not lock @@ -1602,7 +1600,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) RelationGetDescr(oldrelation), oldrc, childOID, NoLock, &appinfo, &childrte, - &childRTindex); + &childRTindex, NULL); Assert(childRTindex > 1); Assert(childrte != NULL); Assert(appinfo != NULL); @@ -1654,7 +1652,8 @@ add_inheritance_child_to_query(PlannerInfo *root, RangeTblEntry *parentrte, Oid childOID, int lockmode, AppendRelInfo **appinfo_p, RangeTblEntry **childrte_p, - Index *childRTindex_p) + Index *childRTindex_p, + PlanRowMark **child_rowmark) { Query *parse = root->parse; Oid parentOID = parentrte->relid; @@ -1807,6 +1806,8 @@ add_inheritance_child_to_query(PlannerInfo *root, RangeTblEntry *parentrte, top_parentrc->allMarkTypes |= childrc->allMarkTypes; root->rowMarks = lappend(root->rowMarks, childrc); + if (child_rowmark) + *child_rowmark = childrc; } /* Close child relations, but keep locks */ diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index d73b4a8499..e98b626395 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1808,16 +1808,18 @@ build_partition_rel(PlannerInfo *root, RelOptInfo *parent, Oid partoid) { RangeTblEntry *parentrte = root->simple_rte_array[parent->relid]; RelOptInfo *result; + Index rootRTindex = 0; Index partRTindex = 0; RangeTblEntry *partrte = NULL; AppendRelInfo *appinfo = NULL; PlanRowMark *rootrc = NULL; + int orig_allMarkTypes; + PlanRowMark *childrc = NULL; + int lockmode; /* Locate the root partitioned table and fetch its PlanRowMark, if any. */ if (root->rowMarks) { - Index rootRTindex = 0; - /* * The root partitioned table itself might be a child of UNION ALL * parent, so we must resort to finding the root parent like this. @@ -1842,16 +1844,27 @@ build_partition_rel(PlannerInfo *root, RelOptInfo *parent, Oid partoid) } rootrc = get_plan_rowmark(root->rowMarks, rootRTindex); + + /* + * Addition of certain child rowmarks will change the value, in which + * case, we need to add additional columns to the query's targetlist. + */ + if (rootrc) + orig_allMarkTypes = rootrc->allMarkTypes; } - /* - * expand_inherited_rtentry alreay locked all partitions, so pass - * NoLock for lockmode. - */ + /* Determine the correct lockmode to use. */ + if (rootRTindex == root->parse->resultRelation) + lockmode = RowExclusiveLock; + else if (rootrc && RowMarkRequiresRowShareLock(rootrc->markType)) + lockmode = RowShareLock; + else + lockmode = AccessShareLock; add_inheritance_child_to_query(root, parentrte, parent->relid, parent->reltype, parent->tupdesc, - rootrc, partoid, NoLock, - &appinfo, &partrte, &partRTindex); + rootrc, partoid, lockmode, + &appinfo, &partrte, &partRTindex, + &childrc); /* Partition turned out to be a partitioned table with 0 partitions. */ if (partrte == NULL) @@ -1870,6 +1883,13 @@ build_partition_rel(PlannerInfo *root, RelOptInfo *parent, Oid partoid) result->lateral_relids = parent->lateral_relids; result->lateral_referencers = parent->lateral_referencers; + if (rootrc && rootrc->allMarkTypes != orig_allMarkTypes) + { + List *tlist = root->parse->targetList; + + tlist = add_rowmark_columns(root, tlist, list_make1(rootrc)); + } + return result; } diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h index ca66f75544..ee7406e4f6 100644 --- a/src/include/optimizer/prep.h +++ b/src/include/optimizer/prep.h @@ -39,6 +39,9 @@ extern Expr *canonicalize_qual(Expr *qual, bool is_check); * prototypes for preptlist.c */ extern List *preprocess_targetlist(PlannerInfo *root); +extern List *add_rowmark_columns(PlannerInfo *root, + List *tlist, + List *rowMarks); extern PlanRowMark *get_plan_rowmark(List *rowmarks, Index rtindex); @@ -57,7 +60,8 @@ extern void add_inheritance_child_to_query(PlannerInfo *root, Oid childOID, int lockmode, AppendRelInfo **appinfo_p, RangeTblEntry **childrte_p, - Index *childRTindex_p); + Index *childRTindex_p, + PlanRowMark **child_rowmark); extern Node *adjust_appendrel_attrs(PlannerInfo *root, Node *node, int nappinfos, AppendRelInfo **appinfos); -- 2.11.0