On Thu, Jun 18, 2015 at 4:04 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > ... although I see that range_table_mutator doesn't bother to copy/change > the column alias substructure. (Wonder if that gives rise to any > observable EXPLAIN bugs...) But it still seems like the append_rel_list > shouldn't be all that much bulkier than all the other crap that gets > generated inside this loop. We're not doing anything at all to reclaim > space consumed inside subquery_planner, and you'd think that would be > a lot. > > By the by, the tablesample additions to range_table_mutator are obviously > broken.
Whee. Meanwhile, here is an updated patch. The attached script (a modified version of something Thomas Munro sent me privately) contains a bunch of test queries. With the original patch I sent earlier, here are the timings I got: Q1 Time: 16215.887 ms Q2 Time: 18674.139 ms Q3 Time: 1029.093 ms Q4 Time: 86497.781 ms Q5 Time: 1143.851 ms This version is about the same for the last three, but the first two get much faster: Q1 Time: 2951.231 ms Q2 Time: 1251.809 ms Q3 Time: 1049.235 ms Q4 Time: 88477.803 ms Q5 Time: 1172.965 ms The speedup comes from the following trick: the first time we hit a query that might requite a ChangeVarNodes() on the append_rel_list, we compute a bitmapset of varnos that appear in that list. Then, every time we're thinking about doing a ChangeVarNodes from rti to new_rti, we check whether rti appears in the Bitmapset. If not, we can skip ChangeVarNodes(). That seems to reduce the amount of object-copying and object-walking attributable to this loop to something negligible in all of these test cases. The extraordinarily planning time for query 4 is caused by a completely different problem: SearchCatCache eats up huge amounts of CPU; its callers are get_attavgwidth and get_typlen. It's not clear to me why doubling the number of relations causes such an enormous explosion in calls to those functions; I would expect the number of calls to double, but presumably the actual increase is much more. That's a separate problem, though, unconnected to c03ad5602f529787968fa3201b35c119bbc6d782 and not necessarily a regression. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
repro-planner-explosion.sh
Description: Bourne shell script
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 8afde2b..6737818 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -43,6 +43,7 @@
#include "parser/parsetree.h"
#include "parser/parse_agg.h"
#include "rewrite/rewriteManip.h"
+#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/selfuncs.h"
@@ -845,9 +846,19 @@ inheritance_planner(PlannerInfo *root)
List *returningLists = NIL;
List *rowMarks;
ListCell *lc;
+ MemoryContext myctx;
+ MemoryContext oldctx;
+ bool did_extract = false;
+ Relids append_rel_list_relids = NULL;
Assert(parse->commandType != CMD_INSERT);
+ myctx = AllocSetContextCreate(CurrentMemoryContext,
+ "inheritance_planner",
+ ALLOCSET_DEFAULT_MINSIZE,
+ ALLOCSET_DEFAULT_INITSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
+
/*
* We generate a modified instance of the original Query for each target
* relation, plan that, and put all the plans into a list that will be
@@ -908,18 +919,16 @@ inheritance_planner(PlannerInfo *root)
/*
* The rowMarks list might contain references to subquery RTEs, so
- * make a copy that we can apply ChangeVarNodes to. (Fortunately, the
- * executor doesn't need to see the modified copies --- we can just
- * pass it the original rowMarks list.)
+ * make a copy that we can apply ChangeVarNodes to. If any security
+ * barrier quals are present, the rowMarks list may be further modified
+ * by grouping_planner. (Fortunately, the executor doesn't need to see
+ * the modified copies --- we can just pass it the original rowMarks
+ * list. For the same reason, we can arrange to throw away the copy
+ * we make here relatively quickly.)
*/
+ oldctx = MemoryContextSwitchTo(myctx);
subroot.rowMarks = (List *) copyObject(root->rowMarks);
-
- /*
- * The append_rel_list likewise might contain references to subquery
- * RTEs (if any subqueries were flattenable UNION ALLs). So prepare
- * to apply ChangeVarNodes to that, too.
- */
- subroot.append_rel_list = (List *) copyObject(root->append_rel_list);
+ MemoryContextSwitchTo(oldctx);
/*
* Add placeholders to the child Query's rangetable list to fill the
@@ -942,6 +951,7 @@ inheritance_planner(PlannerInfo *root)
if (final_rtable != NIL)
{
ListCell *lr;
+ bool did_copy = false;
rti = 1;
foreach(lr, parse->rtable)
@@ -968,7 +978,39 @@ inheritance_planner(PlannerInfo *root)
newrti = list_length(subroot.parse->rtable) + 1;
ChangeVarNodes((Node *) subroot.parse, rti, newrti, 0);
ChangeVarNodes((Node *) subroot.rowMarks, rti, newrti, 0);
- ChangeVarNodes((Node *) subroot.append_rel_list, rti, newrti, 0);
+
+ /*
+ * The append_rel_list likewise might contain references
+ * to subquery RTEs (if any subqueries were flattenable
+ * UNION ALLs). We can't modify the original copy owned
+ * by the parent root, so we have to copy the list if
+ * anything needs to be changed. But that may be expensive
+ * if there are many RTEs, so if we have not yet made a
+ * copy, do so only if ChangeVarNodes will not be a no-op.
+ * Even walking the whole node tree is expensive, so skip
+ * that unless a reference to this RTI is indeed present.
+ */
+ if (!did_extract)
+ {
+ append_rel_list_relids =
+ ExtractRTIndexes((Node *) subroot.append_rel_list,
+ 0);
+ did_extract = true;
+ }
+ if (bms_is_member(rti, append_rel_list_relids))
+ {
+ if (!did_copy)
+ {
+ oldctx = MemoryContextSwitchTo(myctx);
+ subroot.append_rel_list =
+ (List *) copyObject(root->append_rel_list);
+ MemoryContextSwitchTo(oldctx);
+ did_copy = true;
+ }
+ ChangeVarNodes((Node *) subroot.append_rel_list, rti,
+ newrti, 0);
+ }
+
rte = copyObject(rte);
ChangeVarNodes((Node *) rte->securityQuals, rti, newrti, 0);
subroot.parse->rtable = lappend(subroot.parse->rtable,
@@ -976,6 +1018,8 @@ inheritance_planner(PlannerInfo *root)
}
rti++;
}
+
+ bms_free(append_rel_list_relids);
}
/* There shouldn't be any OJ or LATERAL info to translate, as yet */
@@ -989,6 +1033,9 @@ inheritance_planner(PlannerInfo *root)
/* Generate plan */
subplan = grouping_planner(&subroot, 0.0 /* retrieve all tuples */ );
+ /* Reclaim some memory */
+ MemoryContextReset(myctx);
+
/*
* Planning may have modified the query result relation (if there were
* security barrier quals on the result RTE).
@@ -1097,6 +1144,8 @@ inheritance_planner(PlannerInfo *root)
Assert(!parse->onConflict);
}
+ MemoryContextDelete(myctx);
+
/* Mark result as unordered (probably unnecessary) */
root->query_pathkeys = NIL;
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 1da90ff..3075b9a 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -674,6 +674,152 @@ adjust_relid_set(Relids relids, int oldrelid, int newrelid)
}
/*
+ * ExtractRTIndexes - find all RT indexes referenced in a node tree
+ *
+ * This is similar to pull_varnos, but it needs to use exactly the same
+ * criteria as ChangeVarNodes, and pull_varnos does not.
+ */
+
+typedef struct
+{
+ int sublevels_up;
+ Relids varnos;
+} ExtractRTIndexes_context;
+
+static bool
+ExtractRTIndexes_walker(Node *node, ExtractRTIndexes_context *context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ if (var->varlevelsup == context->sublevels_up)
+ context->varnos = bms_add_member(context->varnos, var->varno);
+ return false;
+ }
+ if (IsA(node, CurrentOfExpr))
+ {
+ CurrentOfExpr *cexpr = (CurrentOfExpr *) node;
+
+ if (context->sublevels_up == 0)
+ context->varnos = bms_add_member(context->varnos, cexpr->cvarno);
+ return false;
+ }
+ if (IsA(node, RangeTblRef))
+ {
+ RangeTblRef *rtr = (RangeTblRef *) node;
+
+ if (context->sublevels_up == 0)
+ context->varnos = bms_add_member(context->varnos, rtr->rtindex);
+ /* the subquery itself is visited separately */
+ return false;
+ }
+ if (IsA(node, JoinExpr))
+ {
+ JoinExpr *j = (JoinExpr *) node;
+
+ if (context->sublevels_up == 0)
+ context->varnos = bms_add_member(context->varnos, j->rtindex);
+ /* fall through to examine children */
+ }
+ if (IsA(node, PlaceHolderVar))
+ {
+ PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+ if (phv->phlevelsup == context->sublevels_up)
+ context->varnos = bms_add_members(context->varnos, phv->phrels);
+ /* fall through to examine children */
+ }
+ if (IsA(node, PlanRowMark))
+ {
+ PlanRowMark *rowmark = (PlanRowMark *) node;
+
+ if (context->sublevels_up == 0)
+ {
+ context->varnos = bms_add_member(context->varnos, rowmark->rti);
+ context->varnos = bms_add_member(context->varnos, rowmark->prti);
+ }
+ return false;
+ }
+ if (IsA(node, AppendRelInfo))
+ {
+ AppendRelInfo *appinfo = (AppendRelInfo *) node;
+
+ if (context->sublevels_up == 0)
+ {
+ context->varnos = bms_add_member(context->varnos,
+ appinfo->parent_relid);
+ context->varnos = bms_add_member(context->varnos,
+ appinfo->child_relid);
+ }
+ /* fall through to examine children */
+ }
+ /* Shouldn't need to handle other planner auxiliary nodes here */
+ Assert(!IsA(node, SpecialJoinInfo));
+ Assert(!IsA(node, LateralJoinInfo));
+ Assert(!IsA(node, PlaceHolderInfo));
+ Assert(!IsA(node, MinMaxAggInfo));
+
+ if (IsA(node, Query))
+ {
+ /* Recurse into subselects */
+ bool result;
+
+ context->sublevels_up++;
+ result = query_tree_walker((Query *) node, ExtractRTIndexes_walker,
+ (void *) context, 0);
+ context->sublevels_up--;
+ return result;
+ }
+ return expression_tree_walker(node, ExtractRTIndexes_walker,
+ (void *) context);
+}
+
+Relids
+ExtractRTIndexes(Node *node, int sublevels_up)
+{
+ ExtractRTIndexes_context context;
+
+ context.sublevels_up = sublevels_up;
+ context.varnos = NULL;
+
+ /*
+ * Must be prepared to start with a Query or a bare expression tree; if
+ * it's a Query, go straight to query_tree_walker to make sure that
+ * sublevels_up doesn't get incremented prematurely.
+ */
+ if (node && IsA(node, Query))
+ {
+ Query *qry = (Query *) node;
+
+ if (sublevels_up == 0)
+ {
+ ListCell *l;
+
+ context.varnos = bms_add_member(context.varnos,
+ qry->resultRelation);
+ if (qry->onConflict)
+ context.varnos = bms_add_member(context.varnos,
+ qry->onConflict->exclRelIndex);
+
+ foreach(l, qry->rowMarks)
+ {
+ RowMarkClause *rc = (RowMarkClause *) lfirst(l);
+
+ context.varnos = bms_add_member(context.varnos, rc->rti);
+ }
+ }
+ query_tree_walker(qry, ExtractRTIndexes_walker, (void *) &context, 0);
+ }
+ else
+ ExtractRTIndexes_walker(node, &context);
+
+ return context.varnos;
+}
+
+/*
* IncrementVarSublevelsUp - adjust Var nodes when pushing them down in tree
*
* Find all Var nodes in the given tree having varlevelsup >= min_sublevels_up,
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index bb56fa0..1fc1587 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -15,6 +15,7 @@
#define REWRITEMANIP_H
#include "nodes/parsenodes.h"
+#include "nodes/relation.h"
typedef struct replace_rte_variables_context replace_rte_variables_context;
@@ -42,6 +43,7 @@ typedef enum ReplaceVarsNoMatchOption
extern void OffsetVarNodes(Node *node, int offset, int sublevels_up);
extern void ChangeVarNodes(Node *node, int old_varno, int new_varno,
int sublevels_up);
+extern Relids ExtractRTIndexes(Node *node, int sublevels_up);
extern void IncrementVarSublevelsUp(Node *node, int delta_sublevels_up,
int min_sublevels_up);
extern void IncrementVarSublevelsUp_rtable(List *rtable,
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
