Improve handling of group-column indexes in GroupingSetsPath.

Instead of having planner.c compute a groupColIdx array and store it in
GroupingSetsPaths, make create_groupingsets_plan() find the grouping
columns by searching in the child plan node's tlist.  Although that's
probably a bit slower for create_groupingsets_plan(), it's more like
the way every other plan node type does this, and it provides positive
confirmation that we know which child output columns we're supposed to be
grouping on.  (Indeed, looking at this now, I'm not at all sure that it
wasn't broken before, because create_groupingsets_plan() isn't demanding
an exact tlist match from its child node.)  Also, this allows substantial
simplification in planner.c, because it no longer needs to compute the
groupColIdx array at all; no other cases were using it.

I'd intended to put off this refactoring until later (like 9.7), but
in view of the likely bug fix and the need to rationalize planner.c's
tlist handling so we can do something sane with Konstantin Knizhnik's
function-evaluation-postponement patch, I think it can't wait.

Author: tglsfdc

src/backend/nodes/outfuncs.c

@@ -1875,7 +1875,6 @@ _outGroupingSetsPath(StringInfo str, const GroupingSetsPath *node)
 	_outPathInfo(str, (const Path *) node);
 
 	WRITE_NODE_FIELD(subpath);
-	/* we don't bother to print groupColIdx */
 	WRITE_NODE_FIELD(rollup_groupclauses);
 	WRITE_NODE_FIELD(rollup_lists);
 	WRITE_NODE_FIELD(qual);

src/backend/optimizer/plan/createplan.c

@@ -1640,13 +1640,11 @@ create_groupingsets_plan(PlannerInfo *root, GroupingSetsPath *best_path)
 {
 	Agg		   *plan;
 	Plan	   *subplan;
-	AttrNumber *groupColIdx = best_path->groupColIdx;
 	List	   *rollup_groupclauses = best_path->rollup_groupclauses;
 	List	   *rollup_lists = best_path->rollup_lists;
 	AttrNumber *grouping_map;
 	int			maxref;
 	List	   *chain;
-	int			i;
 	ListCell   *lc,
 			   *lc2;
 
@@ -1662,8 +1660,9 @@ create_groupingsets_plan(PlannerInfo *root, GroupingSetsPath *best_path)
 	subplan = create_plan_recurse(root, best_path->subpath, CP_LABEL_TLIST);
 
 	/*
-	 * Compute the mapping from tleSortGroupRef to column index.  First,
-	 * identify max SortGroupRef in groupClause, for array sizing.
+	 * Compute the mapping from tleSortGroupRef to column index in the child's
+	 * tlist.  First, identify max SortGroupRef in groupClause, for array
+	 * sizing.
 	 */
 	maxref = 0;
 	foreach(lc, root->parse->groupClause)
@@ -1676,12 +1675,13 @@ create_groupingsets_plan(PlannerInfo *root, GroupingSetsPath *best_path)
 
 	grouping_map = (AttrNumber *) palloc0((maxref + 1) * sizeof(AttrNumber));
 
-	i = 0;
+	/* Now look up the column numbers in the child's tlist */
 	foreach(lc, root->parse->groupClause)
 	{
 		SortGroupClause *gc = (SortGroupClause *) lfirst(lc);
+		TargetEntry *tle = get_sortgroupclause_tle(gc, subplan->targetlist);
 
-		grouping_map[gc->tleSortGroupRef] = groupColIdx[i++];
+		grouping_map[gc->tleSortGroupRef] = tle->resno;
 	}
 
 	/*

src/backend/optimizer/plan/planner.c

@@ -104,7 +104,6 @@ static double get_number_of_groups(PlannerInfo *root,
 static RelOptInfo *create_grouping_paths(PlannerInfo *root,
 					  RelOptInfo *input_rel,
 					  PathTarget *target,
-					  AttrNumber *groupColIdx,
 					  List *rollup_lists,
 					  List *rollup_groupclauses);
 static RelOptInfo *create_window_paths(PlannerInfo *root,
@@ -125,9 +124,7 @@ static RelOptInfo *create_distinct_paths(PlannerInfo *root,
 static RelOptInfo *create_ordered_paths(PlannerInfo *root,
 					 RelOptInfo *input_rel,
 					 double limit_tuples);
-static PathTarget *make_scanjoin_target(PlannerInfo *root, List *tlist,
-					 AttrNumber **groupColIdx);
-static int	get_grouping_column_index(Query *parse, TargetEntry *tle);
+static PathTarget *make_scanjoin_target(PlannerInfo *root, List *tlist);
 static List *postprocess_setop_tlist(List *new_tlist, List *orig_tlist);
 static List *select_active_windows(PlannerInfo *root, WindowFuncLists *wflists);
 static List *make_windowInputTargetList(PlannerInfo *root,
@@ -1462,7 +1459,6 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 	{
 		/* No set operations, do regular planning */
 		PathTarget *sub_target;
-		AttrNumber *groupColIdx;
 		double		tlist_rows;
 		List	   *grouping_tlist;
 		WindowFuncLists *wflists = NULL;
@@ -1656,8 +1652,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 		 * because the target width estimates can use per-Var width numbers
 		 * that were obtained within query_planner().
 		 */
-		sub_target = make_scanjoin_target(root, tlist,
-										  &groupColIdx);
+		sub_target = make_scanjoin_target(root, tlist);
 
 		/*
 		 * Forcibly apply that tlist to all the Paths for the scan/join rel.
@@ -1714,7 +1709,6 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 												current_rel,
 												create_pathtarget(root,
 															 grouping_tlist),
-												groupColIdx,
 												rollup_lists,
 												rollup_groupclauses);
 		}
@@ -3077,7 +3071,6 @@ get_number_of_groups(PlannerInfo *root,
  *
  * input_rel: contains the source-data Paths
  * target: the pathtarget for the result Paths to compute
- * groupColIdx: array of indexes of grouping columns in the source data
  * rollup_lists: list of grouping sets, or NIL if not doing grouping sets
  * rollup_groupclauses: list of grouping clauses for grouping sets,
  *		or NIL if not doing grouping sets
@@ -3092,7 +3085,6 @@ static RelOptInfo *
 create_grouping_paths(PlannerInfo *root,
 					  RelOptInfo *input_rel,
 					  PathTarget *target,
-					  AttrNumber *groupColIdx,
 					  List *rollup_lists,
 					  List *rollup_groupclauses)
 {
@@ -3236,7 +3228,6 @@ create_grouping_paths(PlannerInfo *root,
 													  path,
 													  target,
 												  (List *) parse->havingQual,
-													  groupColIdx,
 													  rollup_lists,
 													  rollup_groupclauses,
 													  &agg_costs,
@@ -3766,24 +3757,19 @@ create_ordered_paths(PlannerInfo *root,
  * into PathTarget format, which is more compact and includes cost/width.
  *
  * 'tlist' is the query's target list.
- * 'groupColIdx' receives an array of column numbers for the GROUP BY
- *			expressions (if there are any) in the returned target list.
  *
  * The result is the PathTarget to be applied to the Paths returned from
  * query_planner().
  */
 static PathTarget *
 make_scanjoin_target(PlannerInfo *root,
-					 List *tlist,
-					 AttrNumber **groupColIdx)
+					 List *tlist)
 {
 	Query	   *parse = root->parse;
 	List	   *sub_tlist;
 	List	   *non_group_cols;
 	List	   *non_group_vars;
-	int			numCols;
-
-	*groupColIdx = NULL;
+	ListCell   *tl;
 
 	/*
 	 * If we're not grouping or aggregating or windowing, there's nothing to
@@ -3800,64 +3786,35 @@ make_scanjoin_target(PlannerInfo *root,
 	sub_tlist = NIL;
 	non_group_cols = NIL;
 
-	numCols = list_length(parse->groupClause);
-	if (numCols > 0)
+	foreach(tl, tlist)
 	{
-		/*
-		 * If grouping, create sub_tlist entries for all GROUP BY columns, and
-		 * make an array showing where the group columns are in the sub_tlist.
-		 *
-		 * Note: with this implementation, the array entries will always be
-		 * 1..N, but we don't want callers to assume that.
-		 */
-		AttrNumber *grpColIdx;
-		ListCell   *tl;
-
-		grpColIdx = (AttrNumber *) palloc0(sizeof(AttrNumber) * numCols);
-		*groupColIdx = grpColIdx;
+		TargetEntry *tle = (TargetEntry *) lfirst(tl);
 
-		foreach(tl, tlist)
+		if (tle->ressortgroupref && parse->groupClause &&
+			get_sortgroupref_clause_noerr(tle->ressortgroupref,
+										  parse->groupClause) != NULL)
 		{
-			TargetEntry *tle = (TargetEntry *) lfirst(tl);
-			int			colno;
-
-			colno = get_grouping_column_index(parse, tle);
-			if (colno >= 0)
-			{
-				/*
-				 * It's a grouping column, so add it to the result tlist and
-				 * remember its resno in grpColIdx[].
-				 */
-				TargetEntry *newtle;
-
-				newtle = makeTargetEntry(tle->expr,
-										 list_length(sub_tlist) + 1,
-										 NULL,
-										 false);
-				newtle->ressortgroupref = tle->ressortgroupref;
-				sub_tlist = lappend(sub_tlist, newtle);
+			/*
+			 * It's a grouping column, so add it to the result tlist as-is.
+			 */
+			TargetEntry *newtle;
 
-				Assert(grpColIdx[colno] == 0);	/* no dups expected */
-				grpColIdx[colno] = newtle->resno;
-			}
-			else
-			{
-				/*
-				 * Non-grouping column, so just remember the expression for
-				 * later call to pull_var_clause.  There's no need for
-				 * pull_var_clause to examine the TargetEntry node itself.
-				 */
-				non_group_cols = lappend(non_group_cols, tle->expr);
-			}
+			newtle = makeTargetEntry(tle->expr,
+									 list_length(sub_tlist) + 1,
+									 NULL,
+									 false);
+			newtle->ressortgroupref = tle->ressortgroupref;
+			sub_tlist = lappend(sub_tlist, newtle);
+		}
+		else
+		{
+			/*
+			 * Non-grouping column, so just remember the expression for later
+			 * call to pull_var_clause.  There's no need for pull_var_clause
+			 * to examine the TargetEntry node itself.
+			 */
+			non_group_cols = lappend(non_group_cols, tle->expr);
 		}
-	}
-	else
-	{
-		/*
-		 * With no grouping columns, just pass whole tlist to pull_var_clause.
-		 * Need (shallow) copy to avoid damaging input tlist below.
-		 */
-		non_group_cols = list_copy(tlist);
 	}
 
 	/*
@@ -3886,37 +3843,6 @@ make_scanjoin_target(PlannerInfo *root,
 	return create_pathtarget(root, sub_tlist);
 }
 
-/*
- * get_grouping_column_index
- *		Get the GROUP BY column position, if any, of a targetlist entry.
- *
- * Returns the index (counting from 0) of the TLE in the GROUP BY list, or -1
- * if it's not a grouping column.  Note: the result is unique because the
- * parser won't make multiple groupClause entries for the same TLE.
- */
-static int
-get_grouping_column_index(Query *parse, TargetEntry *tle)
-{
-	int			colno = 0;
-	Index		ressortgroupref = tle->ressortgroupref;
-	ListCell   *gl;
-
-	/* No need to search groupClause if TLE hasn't got a sortgroupref */
-	if (ressortgroupref == 0)
-		return -1;
-
-	foreach(gl, parse->groupClause)
-	{
-		SortGroupClause *grpcl = (SortGroupClause *) lfirst(gl);
-
-		if (grpcl->tleSortGroupRef == ressortgroupref)
-			return colno;
-		colno++;
-	}
-
-	return -1;
-}
-
 /*
  * postprocess_setop_tlist
  *	  Fix up targetlist returned by plan_set_operations().

src/backend/optimizer/util/pathnode.c

@@ -2438,13 +2438,12 @@ create_agg_path(PlannerInfo *root,
  *
  * GroupingSetsPath represents sorted grouping with one or more grouping sets.
  * The input path's result must be sorted to match the last entry in
- * rollup_groupclauses, and groupColIdx[] identifies its sort columns.
+ * rollup_groupclauses.
  *
  * 'rel' is the parent relation associated with the result
  * 'subpath' is the path representing the source of data
  * 'target' is the PathTarget to be computed
  * 'having_qual' is the HAVING quals if any
- * 'groupColIdx' is an array of indexes of grouping columns in the source data
  * 'rollup_lists' is a list of grouping sets
  * 'rollup_groupclauses' is a list of grouping clauses for grouping sets
  * 'agg_costs' contains cost info about the aggregate functions to be computed
@@ -2456,7 +2455,6 @@ create_groupingsets_path(PlannerInfo *root,
 						 Path *subpath,
 						 PathTarget *target,
 						 List *having_qual,
-						 AttrNumber *groupColIdx,
 						 List *rollup_lists,
 						 List *rollup_groupclauses,
 						 const AggClauseCosts *agg_costs,
@@ -2487,7 +2485,6 @@ create_groupingsets_path(PlannerInfo *root,
 	else
 		pathnode->path.pathkeys = NIL;
 
-	pathnode->groupColIdx = groupColIdx;
 	pathnode->rollup_groupclauses = rollup_groupclauses;
 	pathnode->rollup_lists = rollup_lists;
 	pathnode->qual = having_qual;

src/backend/optimizer/util/tlist.c

@@ -444,6 +444,26 @@ get_sortgroupref_clause(Index sortref, List *clauses)
 	return NULL;				/* keep compiler quiet */
 }
 
+/*
+ * get_sortgroupref_clause_noerr
+ *		As above, but return NULL rather than throwing an error if not found.
+ */
+SortGroupClause *
+get_sortgroupref_clause_noerr(Index sortref, List *clauses)
+{
+	ListCell   *l;
+
+	foreach(l, clauses)
+	{
+		SortGroupClause *cl = (SortGroupClause *) lfirst(l);
+
+		if (cl->tleSortGroupRef == sortref)
+			return cl;
+	}
+
+	return NULL;
+}
+
 /*
  * extract_grouping_ops - make an array of the equality operator OIDs
  *		for a SortGroupClause list

src/include/nodes/relation.h

@@ -1311,7 +1311,6 @@ typedef struct GroupingSetsPath
 {
 	Path		path;
 	Path	   *subpath;		/* path representing input source */
-	AttrNumber *groupColIdx;	/* grouping col indexes */
 	List	   *rollup_groupclauses;	/* list of lists of SortGroupClause's */
 	List	   *rollup_lists;	/* parallel list of lists of grouping sets */
 	List	   *qual;			/* quals (HAVING quals), if any */

src/include/optimizer/pathnode.h

@@ -173,7 +173,6 @@ extern GroupingSetsPath *create_groupingsets_path(PlannerInfo *root,
 						 Path *subpath,
 						 PathTarget *target,
 						 List *having_qual,
-						 AttrNumber *groupColIdx,
 						 List *rollup_lists,
 						 List *rollup_groupclauses,
 						 const AggClauseCosts *agg_costs,

src/include/optimizer/tlist.h

@@ -46,6 +46,8 @@ extern List *get_sortgrouplist_exprs(List *sgClauses,
 
 extern SortGroupClause *get_sortgroupref_clause(Index sortref,
 						List *clauses);
+extern SortGroupClause *get_sortgroupref_clause_noerr(Index sortref,
+							  List *clauses);
 
 extern Oid *extract_grouping_ops(List *groupClause);
 extern AttrNumber *extract_grouping_cols(List *groupClause, List *tlist);