I wrote:
> Heikki Linnakangas <hlinnakan...@vmware.com> writes:
>> But do we really need to backpatch any of this?

> Alexey's example consumes only a couple hundred MB in 9.2, vs about 7GB
> peak in 9.3 and up.  That seems like a pretty nasty regression.

I did a bit more measurement of the time and backend memory consumption
for Alexey's example EXPLAIN:

9.2: 0.9 sec, circa 200 MB
HEAD: 56 sec, circa 7300 MB
with patch below: 3.7 sec, circa 300 MB

So while this doesn't get us all the way back down to where we were before
we started trying to guarantee unique table/column identifiers in EXPLAIN
printouts, it's at least a lot closer.

Not sure whether to just commit this to HEAD and call it a day, or to
risk back-patching.

It occurred to me that we could use your idea of a secondary data
structure and minimize the code impact with a macro layer, ie
        #define grouping_stack pointer_field->groupingstack
But I'm not sure if it's worth the trouble.  9.3 has been like this
right along, and this is the first complaint.

One compromise idea would be to back-patch only as far as 9.4.
If there are extensions out there that have this ABI issue, expecting
them to rebuild for 9.4.1 might be OK.

                        regards, tom lane

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 8a0be5d..39ceaf2 100644
*** a/src/backend/commands/explain.c
--- b/src/backend/commands/explain.c
*************** ExplainPrintPlan(ExplainState *es, Query
*** 563,568 ****
--- 563,570 ----
  	es->rtable = queryDesc->plannedstmt->rtable;
  	ExplainPreScanNode(queryDesc->planstate, &rels_used);
  	es->rtable_names = select_rtable_names_for_explain(es->rtable, rels_used);
+ 	es->deparse_cxt = deparse_context_for_plan_rtable(es->rtable,
+ 													  es->rtable_names);
  	ExplainNode(queryDesc->planstate, NIL, NULL, NULL, es);
  }
  
*************** show_plan_tlist(PlanState *planstate, Li
*** 1678,1687 ****
  		return;
  
  	/* Set up deparsing context */
! 	context = deparse_context_for_planstate((Node *) planstate,
! 											ancestors,
! 											es->rtable,
! 											es->rtable_names);
  	useprefix = list_length(es->rtable) > 1;
  
  	/* Deparse each result column (we now include resjunk ones) */
--- 1680,1688 ----
  		return;
  
  	/* Set up deparsing context */
! 	context = set_deparse_context_planstate(es->deparse_cxt,
! 											(Node *) planstate,
! 											ancestors);
  	useprefix = list_length(es->rtable) > 1;
  
  	/* Deparse each result column (we now include resjunk ones) */
*************** show_expression(Node *node, const char *
*** 1710,1719 ****
  	char	   *exprstr;
  
  	/* Set up deparsing context */
! 	context = deparse_context_for_planstate((Node *) planstate,
! 											ancestors,
! 											es->rtable,
! 											es->rtable_names);
  
  	/* Deparse the expression */
  	exprstr = deparse_expression(node, context, useprefix, false);
--- 1711,1719 ----
  	char	   *exprstr;
  
  	/* Set up deparsing context */
! 	context = set_deparse_context_planstate(es->deparse_cxt,
! 											(Node *) planstate,
! 											ancestors);
  
  	/* Deparse the expression */
  	exprstr = deparse_expression(node, context, useprefix, false);
*************** show_sort_group_keys(PlanState *planstat
*** 1855,1864 ****
  		return;
  
  	/* Set up deparsing context */
! 	context = deparse_context_for_planstate((Node *) planstate,
! 											ancestors,
! 											es->rtable,
! 											es->rtable_names);
  	useprefix = (list_length(es->rtable) > 1 || es->verbose);
  
  	for (keyno = 0; keyno < nkeys; keyno++)
--- 1855,1863 ----
  		return;
  
  	/* Set up deparsing context */
! 	context = set_deparse_context_planstate(es->deparse_cxt,
! 											(Node *) planstate,
! 											ancestors);
  	useprefix = (list_length(es->rtable) > 1 || es->verbose);
  
  	for (keyno = 0; keyno < nkeys; keyno++)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index dd748ac..cacd72c 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
*************** deparse_context_for(const char *aliasnam
*** 2520,2526 ****
  }
  
  /*
!  * deparse_context_for_planstate	- Build deparse context for a plan
   *
   * When deparsing an expression in a Plan tree, we might have to resolve
   * OUTER_VAR, INNER_VAR, or INDEX_VAR references.  To do this, the caller must
--- 2520,2562 ----
  }
  
  /*
!  * deparse_context_for_plan_rtable - Build deparse context for a plan's rtable
!  *
!  * When deparsing an expression in a Plan tree, we use the plan's rangetable
!  * to resolve names of simple Vars.  The initialization of column names for
!  * this is rather expensive if the rangetable is large, and it'll be the same
!  * for every expression in the Plan tree; so we do it just once and re-use
!  * the result of this function for each expression.  (Note that the result
!  * is not usable until set_deparse_context_planstate() is applied to it.)
!  *
!  * In addition to the plan's rangetable list, pass the per-RTE alias names
!  * assigned by a previous call to select_rtable_names_for_explain.
!  */
! List *
! deparse_context_for_plan_rtable(List *rtable, List *rtable_names)
! {
! 	deparse_namespace *dpns;
! 
! 	dpns = (deparse_namespace *) palloc0(sizeof(deparse_namespace));
! 
! 	/* Initialize fields that stay the same across the whole plan tree */
! 	dpns->rtable = rtable;
! 	dpns->rtable_names = rtable_names;
! 	dpns->ctes = NIL;
! 
! 	/*
! 	 * Set up column name aliases.  We will get rather bogus results for join
! 	 * RTEs, but that doesn't matter because plan trees don't contain any join
! 	 * alias Vars.
! 	 */
! 	set_simple_column_names(dpns);
! 
! 	/* Return a one-deep namespace stack */
! 	return list_make1(dpns);
! }
! 
! /*
!  * set_deparse_context_planstate	- Specify Plan node containing expression
   *
   * When deparsing an expression in a Plan tree, we might have to resolve
   * OUTER_VAR, INNER_VAR, or INDEX_VAR references.  To do this, the caller must
*************** deparse_context_for(const char *aliasnam
*** 2539,2575 ****
   * most-closely-nested first.  This is needed to resolve PARAM_EXEC Params.
   * Note we assume that all the PlanStates share the same rtable.
   *
!  * The plan's rangetable list must also be passed, along with the per-RTE
!  * alias names assigned by a previous call to select_rtable_names_for_explain.
!  * (We use the rangetable to resolve simple Vars, but the plan inputs are
!  * necessary for Vars with special varnos.)
   */
  List *
! deparse_context_for_planstate(Node *planstate, List *ancestors,
! 							  List *rtable, List *rtable_names)
  {
  	deparse_namespace *dpns;
  
! 	dpns = (deparse_namespace *) palloc0(sizeof(deparse_namespace));
! 
! 	/* Initialize fields that stay the same across the whole plan tree */
! 	dpns->rtable = rtable;
! 	dpns->rtable_names = rtable_names;
! 	dpns->ctes = NIL;
! 
! 	/*
! 	 * Set up column name aliases.  We will get rather bogus results for join
! 	 * RTEs, but that doesn't matter because plan trees don't contain any join
! 	 * alias Vars.
! 	 */
! 	set_simple_column_names(dpns);
  
  	/* Set our attention on the specific plan node passed in */
  	set_deparse_planstate(dpns, (PlanState *) planstate);
  	dpns->ancestors = ancestors;
  
! 	/* Return a one-deep namespace stack */
! 	return list_make1(dpns);
  }
  
  /*
--- 2575,2602 ----
   * most-closely-nested first.  This is needed to resolve PARAM_EXEC Params.
   * Note we assume that all the PlanStates share the same rtable.
   *
!  * Once this function has been called, deparse_expression() can be called on
!  * subsidiary expression(s) of the specified PlanState node.  To deparse
!  * expressions of a different Plan node in the same Plan tree, re-call this
!  * function to identify the new parent Plan node.
!  *
!  * The result is the same List passed in; this is a notational convenience.
   */
  List *
! set_deparse_context_planstate(List *dpcontext,
! 							  Node *planstate, List *ancestors)
  {
  	deparse_namespace *dpns;
  
! 	/* Should always have one-entry namespace list for Plan deparsing */
! 	Assert(list_length(dpcontext) == 1);
! 	dpns = (deparse_namespace *) linitial(dpcontext);
  
  	/* Set our attention on the specific plan node passed in */
  	set_deparse_planstate(dpns, (PlanState *) planstate);
  	dpns->ancestors = ancestors;
  
! 	return dpcontext;
  }
  
  /*
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 6e26950..598f374 100644
*** a/src/include/commands/explain.h
--- b/src/include/commands/explain.h
*************** typedef struct ExplainState
*** 41,46 ****
--- 41,47 ----
  	List	   *rtable_names;	/* alias names for RTEs */
  	int			indent;			/* current indentation level */
  	List	   *grouping_stack; /* format-specific grouping state */
+ 	List	   *deparse_cxt;	/* context list for deparsing expressions */
  } ExplainState;
  
  /* Hook for plugins to get control in ExplainOneQuery() */
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index 188503c..86d2bea 100644
*** a/src/include/utils/ruleutils.h
--- b/src/include/utils/ruleutils.h
*************** extern char *pg_get_constraintdef_string
*** 25,32 ****
  extern char *deparse_expression(Node *expr, List *dpcontext,
  				   bool forceprefix, bool showimplicit);
  extern List *deparse_context_for(const char *aliasname, Oid relid);
! extern List *deparse_context_for_planstate(Node *planstate, List *ancestors,
! 							  List *rtable, List *rtable_names);
  extern List *select_rtable_names_for_explain(List *rtable,
  								Bitmapset *rels_used);
  extern char *generate_collation_name(Oid collid);
--- 25,33 ----
  extern char *deparse_expression(Node *expr, List *dpcontext,
  				   bool forceprefix, bool showimplicit);
  extern List *deparse_context_for(const char *aliasname, Oid relid);
! extern List *deparse_context_for_plan_rtable(List *rtable, List *rtable_names);
! extern List *set_deparse_context_planstate(List *dpcontext,
! 							  Node *planstate, List *ancestors);
  extern List *select_rtable_names_for_explain(List *rtable,
  								Bitmapset *rels_used);
  extern char *generate_collation_name(Oid collid);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to