Fix outer query's target list, when creating a synthetic subquery.

When the planner creates a subquery as part of grouping planning, fix
the target list of the outer query to correctly refer to the outputs of
the subquery. Previously, the parse tree's target list was sometimes left
unchanged, so that it still contained Vars referring to the original
relations, which have been pushed down to the subquery. After adding
the subquery, the outer query only contains a single RTE for the subquery.

We usually got away with a bogus target list in the outer query, because
we had already decided how to construct the plan, and usually didn't look
at the original parse tree anymore. In some cases, though, when the caller
asked for it by passing 'use_root=true', we were already doing the right
thing, but otherwise we didn't bother. There's little downside to fixing
the target list, though, so let's do it unconditionally.

Fixes https://github.com/greenplum-db/gpdb/issues/6754Reviewed-by: NMelanie Plageman <mplageman@pivotal.io>

src/backend/cdb/cdbgroup.c

@@ -1545,7 +1545,6 @@ make_two_stage_agg_plan(PlannerInfo *root,
 									   "partial_aggregation",
 									   &current_pathkeys,
 									   result_plan,
-									   !ctx->is_grpext,
 									   true);
 
 	if (ctx->is_grpext)
@@ -2247,7 +2246,7 @@ make_plan_for_one_dqa(PlannerInfo *root, MppGroupContext *ctx, int dqa_index,
 										   "partial_aggregation",
 										   &current_pathkeys,
 										   result_plan,
-										   true, false);
+										   false);
 	}
 
 	/*
@@ -2324,7 +2323,6 @@ make_plan_for_one_dqa(PlannerInfo *root, MppGroupContext *ctx, int dqa_index,
 									   "partial_aggregation",
 									   &current_pathkeys,
 									   result_plan,
-									   true,
 									   false);
 
 	/* Final sort */
@@ -3621,13 +3619,18 @@ deconstruct_agg_info(MppGroupContext *ctx)
 	{
 		TargetEntry *sub_tle,
 				   *prelim_tle;
+		char	   *resname;
 
 		sub_tle = get_tle_by_resno(ctx->sub_tlist, ctx->groupColIdx[i]);
+
+		if (sub_tle->resname)
+			resname = pstrdup(sub_tle->resname);
+		else
+			resname = psprintf("prelim_aggref_%d", sub_tle->ressortgroupref);
+
 		prelim_tle = makeTargetEntry(copyObject(sub_tle->expr),
 									 list_length(ctx->grps_tlist) + 1,
-									 (sub_tle->resname == NULL) ?
-									 NULL :
-									 pstrdup(sub_tle->resname),
+									 resname,
 									 false);
 		prelim_tle->ressortgroupref = sub_tle->ressortgroupref;
 		prelim_tle->resjunk = false;
@@ -4477,7 +4480,6 @@ add_second_stage_agg(PlannerInfo *root,
 					 const char *alias,
 					 List **p_current_pathkeys,
 					 Plan *result_plan,
-					 bool use_root,
 					 bool adjust_scatter)
 {
 	Query	   *parse = root->parse;
@@ -4539,7 +4541,7 @@ add_second_stage_agg(PlannerInfo *root,
 			tle->resjunk = false;
 			if (tle->resname == NULL)
 			{
-				if (use_root && IsA(tle->expr, Var))
+				if (IsA(tle->expr, Var))
 				{
 					Var		   *var = (Var *) tle->expr;
 					RangeTblEntry *rte = rt_fetch(var->varno, root->parse->rtable);
@@ -4586,18 +4588,13 @@ add_second_stage_agg(PlannerInfo *root,
 	parse->rowMarks = NIL;
 
 	/*
-	 * <EXECUTE s> uses parse->targetList to derive the portal's tupDesc, so
-	 * when use_root is true, the caller owns the responsibility to make sure
-	 * it ends up in an appropriate form at the end of planning.
+	 * Set the target list of outer Query tree to match the new range table.
 	 */
-	if (use_root)
+	if (adjust_scatter)
 	{
-		if (adjust_scatter)
-		{
-			UpdateScatterClause(parse, upper_tlist);
-		}
-		parse->targetList = copyObject(upper_tlist);	/* Match range. */
+		UpdateScatterClause(parse, upper_tlist);
 	}
+	parse->targetList = copyObject(upper_tlist);	/* Match range. */
 
 	result_plan = add_subqueryscan(root, p_current_pathkeys,
 								   1, subquery, result_plan);

src/backend/optimizer/plan/plangroupext.c

@@ -891,7 +891,7 @@ make_list_aggs_for_rollup(PlannerInfo *root,
 										context->agg_costs,
 										"rollup",
 										&context->current_pathkeys,
-										agg_node, false, false);
+										agg_node, false);
 
 		/* Set inputHasGrouping */
 		((Agg *)agg_node)->inputHasGrouping = true;
@@ -1001,7 +1001,6 @@ make_list_aggs_for_rollup(PlannerInfo *root,
 										context->agg_costs,
 										"rollup", &context->current_pathkeys,
 										agg_node,
-										false,
 										false);
 
 		if (context->aggstrategy == AGG_HASHED)

src/include/cdb/cdbgroup.h

@@ -64,7 +64,6 @@ extern Plan *add_second_stage_agg(PlannerInfo *root,
 								  const char *alias,
 								  List **p_current_pathkeys,
 								  Plan *result_plan,
-								  bool use_root,
 								  bool adjust_scatter);
 extern List *generate_subquery_tlist(Index varno, List *input_tlist,
 									 bool keep_resjunk, int **p_resno_map);

src/test/regress/expected/bfv_olap.out

@@ -512,6 +512,24 @@ GROUP BY ROLLUP( (sale.dt,sale.cn),(sale.pn),(sale.vn));
    
 (6 rows)
 
+--
+-- Another ROLLUP query, that hit a bug in setting up the planner-generated
+-- subquery's targetlist. (https://github.com/greenplum-db/gpdb/issues/6754)
+--
+SELECT sale.vn, rank() over (partition by sale.vn)
+FROM vendor, sale
+WHERE sale.vn=vendor.vn
+GROUP BY ROLLUP( sale.vn);
+ vn | rank 
+----+------
+    |    1
+ 10 |    1
+ 20 |    1
+ 30 |    1
+ 40 |    1
+ 50 |    1
+(6 rows)
+
 --
 -- Test window function with constant PARTITION BY
 --

src/test/regress/sql/bfv_olap.sql

@@ -347,6 +347,16 @@ WHERE sale.vn=vendor.vn
 GROUP BY ROLLUP( (sale.dt,sale.cn),(sale.pn),(sale.vn));
 
 
+--
+-- Another ROLLUP query, that hit a bug in setting up the planner-generated
+-- subquery's targetlist. (https://github.com/greenplum-db/gpdb/issues/6754)
+--
+SELECT sale.vn, rank() over (partition by sale.vn)
+FROM vendor, sale
+WHERE sale.vn=vendor.vn
+GROUP BY ROLLUP( sale.vn);
+
+
 --
 -- Test window function with constant PARTITION BY
 --