Fix build_grouping_chain() to not clobber its input lists.

There's no good reason for stomping on the input data; it makes the logic
in this function no simpler, in fact probably the reverse.  And it makes
it impossible to separate path generation from plan generation, as I'm
working towards doing; that will require more than one traversal of these
lists.

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index fed1acc684f655f9297f40cb0b2d175500f103de..131dc8a7b1aa1854f9110b072343af5fc33afebe 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -2035,13 +2035,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
                                                                                                   &agg_costs,
                                                                                                   numGroups,
                                                                                                   result_plan);
-
-                               /*
-                                * these are destroyed by build_grouping_chain, so make sure
-                                * we don't try and touch them again
-                                */
-                               rollup_groupclauses = NIL;
-                               rollup_lists = NIL;
                        }
                        else if (parse->groupClause)
                        {
@@ -2461,10 +2454,10 @@ remap_groupColIdx(PlannerInfo *root, List *groupClause)
 
 /*
  * Build Agg and Sort nodes to implement sorted grouping with one or more
- * grouping sets. (A plain GROUP BY or just the presence of aggregates counts
+ * grouping sets.  A plain GROUP BY or just the presence of aggregates counts
  * for this purpose as a single grouping set; the calling code is responsible
- * for providing a non-empty rollup_groupclauses list for such cases, though
- * rollup_lists may be null.)
+ * for providing a single-element rollup_groupclauses list for such cases,
+ * though rollup_lists may be nil.
  *
  * The last entry in rollup_groupclauses (which is the one the input is sorted
  * on, if at all) is the one used for the returned Agg node. Any additional
@@ -2473,8 +2466,6 @@ remap_groupColIdx(PlannerInfo *root, List *groupClause)
  * participate in the plan directly, but they are both a convenient way to
  * represent the required data and a convenient way to account for the costs
  * of execution.
- *
- * rollup_groupclauses and rollup_lists are destroyed by this function.
  */
 static Plan *
 build_grouping_chain(PlannerInfo *root,
@@ -2514,62 +2505,71 @@ build_grouping_chain(PlannerInfo *root,
         * Generate the side nodes that describe the other sort and group
         * operations besides the top one.
         */
-       while (list_length(rollup_groupclauses) > 1)
+       if (list_length(rollup_groupclauses) > 1)
        {
-               List       *groupClause = linitial(rollup_groupclauses);
-               List       *gsets = linitial(rollup_lists);
-               AttrNumber *new_grpColIdx;
-               Plan       *sort_plan;
-               Plan       *agg_plan;
-
-               Assert(groupClause);
-               Assert(gsets);
-
-               new_grpColIdx = remap_groupColIdx(root, groupClause);
+               ListCell   *lc,
+                                  *lc2;
 
-               sort_plan = (Plan *)
-                       make_sort_from_groupcols(root,
-                                                                        groupClause,
-                                                                        new_grpColIdx,
-                                                                        result_plan);
-
-               /*
-                * sort_plan includes the cost of result_plan over again, which is not
-                * what we want (since it's not actually running that plan). So
-                * correct the cost figures.
-                */
+               Assert(list_length(rollup_groupclauses) == list_length(rollup_lists));
+               forboth(lc, rollup_groupclauses, lc2, rollup_lists)
+               {
+                       List       *groupClause = (List *) lfirst(lc);
+                       List       *gsets = (List *) lfirst(lc2);
+                       AttrNumber *new_grpColIdx;
+                       Plan       *sort_plan;
+                       Plan       *agg_plan;
+
+                       /* We want to iterate over all but the last rollup list elements */
+                       if (lnext(lc) == NULL)
+                               break;
 
-               sort_plan->startup_cost -= result_plan->total_cost;
-               sort_plan->total_cost -= result_plan->total_cost;
+                       new_grpColIdx = remap_groupColIdx(root, groupClause);
 
-               agg_plan = (Plan *) make_agg(root,
-                                                                        tlist,
-                                                                        (List *) parse->havingQual,
-                                                                        AGG_SORTED,
-                                                                        agg_costs,
-                                                                        list_length(linitial(gsets)),
-                                                                        new_grpColIdx,
-                                                                        extract_grouping_ops(groupClause),
-                                                                        gsets,
-                                                                        numGroups,
-                                                                        sort_plan);
+                       sort_plan = (Plan *)
+                               make_sort_from_groupcols(root,
+                                                                                groupClause,
+                                                                                new_grpColIdx,
+                                                                                result_plan);
 
-               sort_plan->lefttree = NULL;
+                       /*
+                        * sort_plan includes the cost of result_plan, which is not what
+                        * we want (since we'll not actually run that plan again).  So
+                        * correct the cost figures.
+                        */
+                       sort_plan->startup_cost -= result_plan->total_cost;
+                       sort_plan->total_cost -= result_plan->total_cost;
+
+                       agg_plan = (Plan *) make_agg(root,
+                                                                                tlist,
+                                                                                (List *) parse->havingQual,
+                                                                                AGG_SORTED,
+                                                                                agg_costs,
+                                                                                list_length(linitial(gsets)),
+                                                                                new_grpColIdx,
+                                                                                extract_grouping_ops(groupClause),
+                                                                                gsets,
+                                                                                numGroups,
+                                                                                sort_plan);
 
-               chain = lappend(chain, agg_plan);
+                       /*
+                        * Nuke stuff we don't need to avoid bloating debug output.
+                        */
+                       sort_plan->targetlist = NIL;
+                       sort_plan->lefttree = NULL;
 
-               if (rollup_lists)
-                       rollup_lists = list_delete_first(rollup_lists);
+                       agg_plan->targetlist = NIL;
+                       agg_plan->qual = NIL;
 
-               rollup_groupclauses = list_delete_first(rollup_groupclauses);
+                       chain = lappend(chain, agg_plan);
+               }
        }
 
        /*
         * Now make the final Agg node
         */
        {
-               List       *groupClause = linitial(rollup_groupclauses);
-               List       *gsets = rollup_lists ? linitial(rollup_lists) : NIL;
+               List       *groupClause = (List *) llast(rollup_groupclauses);
+               List       *gsets = rollup_lists ? (List *) llast(rollup_lists) : NIL;
                int                     numGroupCols;
                ListCell   *lc;
 
@@ -2601,14 +2601,6 @@ build_grouping_chain(PlannerInfo *root,
                        Plan       *subplan = lfirst(lc);
 
                        result_plan->total_cost += subplan->total_cost;
-
-                       /*
-                        * Nuke stuff we don't need to avoid bloating debug output.
-                        */
-
-                       subplan->targetlist = NIL;
-                       subplan->qual = NIL;
-                       subplan->lefttree->targetlist = NIL;
                }
        }