Delay creation of subplan tlist until after create_plan().

Once upon a time it was necessary for grouping_planner() to determine
the tlist it wanted from the scan/join plan subtree before it called
query_planner(), because query_planner() would actually make a Plan using
that.  But we refactored things a long time ago to delay construction of
the Plan tree till later, so there's no need to build that tlist until
(and indeed unless) we're ready to plaster it onto the Plan.  The only
thing query_planner() cares about is what Vars are going to be needed for
the tlist, and it can perfectly well get that by looking at the real tlist
rather than some masticated version.

Well, actually, there is one minor glitch in that argument, which is that
make_subplanTargetList also adds Vars appearing only in HAVING to the
tlist it produces.  So now we have to account for HAVING explicitly in
build_base_rel_tlists.  But that just adds a few lines of code, and
I doubt it moves the needle much on processing time; we might be doing
pull_var_clause() twice on the havingQual, but before we had it scanning
dummy tlist entries instead.

This is a very small down payment on rationalizing grouping_planner
enough so it can be refactored.

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index efd40b2022e1fa1e8f26cbc2ddec08efd20b5e57..4a906a88a8577d5a42da16698a82a3247a8a45b7 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -137,7 +137,7 @@ add_base_rels_to_query(PlannerInfo *root, Node *jtnode)
 /*
  * build_base_rel_tlists
  *       Add targetlist entries for each var needed in the query's final tlist
- *       to the appropriate base relations.
+ *       (and HAVING clause, if any) to the appropriate base relations.
  *
  * We mark such vars as needed by "relation 0" to ensure that they will
  * propagate up through all join plan steps.
@@ -154,6 +154,23 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
                add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0), true);
                list_free(tlist_vars);
        }
+
+       /*
+        * If there's a HAVING clause, we'll need the Vars it uses, too.
+        */
+       if (root->parse->havingQual)
+       {
+               List       *having_vars = pull_var_clause(root->parse->havingQual,
+                                                                                                 PVC_RECURSE_AGGREGATES,
+                                                                                                 PVC_INCLUDE_PLACEHOLDERS);
+
+               if (having_vars != NIL)
+               {
+                       add_vars_to_targetlist(root, having_vars,
+                                                                  bms_make_singleton(0), true);
+                       list_free(having_vars);
+               }
+       }
 }
 
 /*

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 7f51d8ce4d895bbe35e4d132dbe7eaa9b5cf5495..a4357639dc5bd4204956814a13e2835833f06525 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1415,9 +1415,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
        else
        {
                /* No set operations, do regular planning */
-               List       *sub_tlist;
-               AttrNumber *groupColIdx = NULL;
-               bool            need_tlist_eval = true;
                long            numGroups = 0;
                AggClauseCosts agg_costs;
                int                     numGroupCols;
@@ -1548,13 +1545,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
                                parse->hasWindowFuncs = false;
                }
 
-               /*
-                * Generate appropriate target list for subplan; may be different from
-                * tlist if grouping or aggregation is needed.
-                */
-               sub_tlist = make_subplanTargetList(root, tlist,
-                                                                                  &groupColIdx, &need_tlist_eval);
-
                /*
                 * Do aggregate preprocessing, if the query has any aggs.
                 *
@@ -1612,7 +1602,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
                 * standard_qp_callback) pathkey representations of the query's sort
                 * clause, distinct clause, etc.
                 */
-               final_rel = query_planner(root, sub_tlist,
+               final_rel = query_planner(root, tlist,
                                                                  standard_qp_callback, &qp_extra);
 
                /*
@@ -1888,6 +1878,9 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
                         * Normal case --- create a plan according to query_planner's
                         * results.
                         */
+                       List       *sub_tlist;
+                       AttrNumber *groupColIdx = NULL;
+                       bool            need_tlist_eval = true;
                        bool            need_sort_for_grouping = false;
 
                        result_plan = create_plan(root, best_path);
@@ -1896,23 +1889,27 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
                        /* Detect if we'll need an explicit sort for grouping */
                        if (parse->groupClause && !use_hashed_grouping &&
                          !pathkeys_contained_in(root->group_pathkeys, current_pathkeys))
-                       {
                                need_sort_for_grouping = true;
 
-                               /*
-                                * Always override create_plan's tlist, so that we don't sort
-                                * useless data from a "physical" tlist.
-                                */
-                               need_tlist_eval = true;
-                       }
+                       /*
+                        * Generate appropriate target list for scan/join subplan; may be
+                        * different from tlist if grouping or aggregation is needed.
+                        */
+                       sub_tlist = make_subplanTargetList(root, tlist,
+                                                                                          &groupColIdx,
+                                                                                          &need_tlist_eval);
 
                        /*
                         * create_plan returns a plan with just a "flat" tlist of required
                         * Vars.  Usually we need to insert the sub_tlist as the tlist of
                         * the top plan node.  However, we can skip that if we determined
                         * that whatever create_plan chose to return will be good enough.
+                        *
+                        * If we need_sort_for_grouping, always override create_plan's
+                        * tlist, so that we don't sort useless data from a "physical"
+                        * tlist.
                         */
-                       if (need_tlist_eval)
+                       if (need_tlist_eval || need_sort_for_grouping)
                        {
                                /*
                                 * If the top-level plan node is one that cannot do expression