Correctly assess parallel-safety of tlists when SRFs are used.

Since commit 69f4b9c85f168ae006929eec44fc44d569e846b9, the existing
code was no longer assessing the parallel-safety of the real tlist
for each upper rel, but rather the first of possibly several tlists
created by split_pathtarget_at_srfs().  Repair.

Even though this is clearly wrong, it's not clear that it has any
user-visible consequences at the moment, so no back-patch for now.  If
we discover later that it does have user-visible consequences, we
might need to back-patch this to v10.

Patch by me, per a report from Rajkumar Raghuwanshi.

Discussion: http://postgr.es/m/CA+Tgmoaob_Strkg4Dcx=VyxnyXtrmkV=ofj=pX7gH9hSre-g0Q@mail.gmail.com

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index de1257d9c223d99ed2ee652c5cadfcc5990105f4..14b7becf3e84da598774312d0c39b817dff56815 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -137,6 +137,7 @@ static Size estimate_hashagg_tablesize(Path *path,
 static RelOptInfo *create_grouping_paths(PlannerInfo *root,
                                          RelOptInfo *input_rel,
                                          PathTarget *target,
+                                         bool target_parallel_safe,
                                          const AggClauseCosts *agg_costs,
                                          grouping_sets_data *gd);
 static void consider_groupingsets_paths(PlannerInfo *root,
@@ -152,6 +153,7 @@ static RelOptInfo *create_window_paths(PlannerInfo *root,
                                        RelOptInfo *input_rel,
                                        PathTarget *input_target,
                                        PathTarget *output_target,
+                                       bool output_target_parallel_safe,
                                        List *tlist,
                                        WindowFuncLists *wflists,
                                        List *activeWindows);
@@ -168,6 +170,7 @@ static RelOptInfo *create_distinct_paths(PlannerInfo *root,
 static RelOptInfo *create_ordered_paths(PlannerInfo *root,
                                         RelOptInfo *input_rel,
                                         PathTarget *target,
+                                        bool target_parallel_safe,
                                         double limit_tuples);
 static PathTarget *make_group_input_target(PlannerInfo *root,
                                                PathTarget *final_target);
@@ -1583,6 +1586,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
        PathTarget *final_target;
        List       *final_targets;
        List       *final_targets_contain_srfs;
+       bool            final_target_parallel_safe;
        RelOptInfo *current_rel;
        RelOptInfo *final_rel;
        ListCell   *lc;
@@ -1645,6 +1649,10 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                /* Also extract the PathTarget form of the setop result tlist */
                final_target = current_rel->cheapest_total_path->pathtarget;
 
+               /* And check whether it's parallel safe */
+               final_target_parallel_safe =
+                       is_parallel_safe(root, (Node *) final_target->exprs);
+
                /* The setop result tlist couldn't contain any SRFs */
                Assert(!parse->hasTargetSRFs);
                final_targets = final_targets_contain_srfs = NIL;
@@ -1676,12 +1684,15 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                PathTarget *sort_input_target;
                List       *sort_input_targets;
                List       *sort_input_targets_contain_srfs;
+               bool            sort_input_target_parallel_safe;
                PathTarget *grouping_target;
                List       *grouping_targets;
                List       *grouping_targets_contain_srfs;
+               bool            grouping_target_parallel_safe;
                PathTarget *scanjoin_target;
                List       *scanjoin_targets;
                List       *scanjoin_targets_contain_srfs;
+               bool            scanjoin_target_parallel_safe;
                bool            have_grouping;
                AggClauseCosts agg_costs;
                WindowFuncLists *wflists = NULL;
@@ -1805,6 +1816,8 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                 * that were obtained within query_planner().
                 */
                final_target = create_pathtarget(root, tlist);
+               final_target_parallel_safe =
+                       is_parallel_safe(root, (Node *) final_target->exprs);
 
                /*
                 * If ORDER BY was given, consider whether we should use a post-sort
@@ -1812,11 +1825,18 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                 * so.
                 */
                if (parse->sortClause)
+               {
                        sort_input_target = make_sort_input_target(root,
                                                                                                           final_target,
                                                                                                           &have_postponed_srfs);
+                       sort_input_target_parallel_safe =
+                               is_parallel_safe(root, (Node *) sort_input_target->exprs);
+               }
                else
+               {
                        sort_input_target = final_target;
+                       sort_input_target_parallel_safe = final_target_parallel_safe;
+               }
 
                /*
                 * If we have window functions to deal with, the output from any
@@ -1824,11 +1844,18 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                 * otherwise, it should be sort_input_target.
                 */
                if (activeWindows)
+               {
                        grouping_target = make_window_input_target(root,
                                                                                                           final_target,
                                                                                                           activeWindows);
+                       grouping_target_parallel_safe =
+                               is_parallel_safe(root, (Node *) grouping_target->exprs);
+               }
                else
+               {
                        grouping_target = sort_input_target;
+                       grouping_target_parallel_safe = sort_input_target_parallel_safe;
+               }
 
                /*
                 * If we have grouping or aggregation to do, the topmost scan/join
@@ -1838,9 +1865,16 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                have_grouping = (parse->groupClause || parse->groupingSets ||
                                                 parse->hasAggs || root->hasHavingQual);
                if (have_grouping)
+               {
                        scanjoin_target = make_group_input_target(root, final_target);
+                       scanjoin_target_parallel_safe =
+                               is_parallel_safe(root, (Node *) grouping_target->exprs);
+               }
                else
+               {
                        scanjoin_target = grouping_target;
+                       scanjoin_target_parallel_safe = grouping_target_parallel_safe;
+               }
 
                /*
                 * If there are any SRFs in the targetlist, we must separate each of
@@ -1922,8 +1956,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                 * for partial paths.  But only parallel-safe expressions can be
                 * computed by partial paths.
                 */
-               if (current_rel->partial_pathlist &&
-                       is_parallel_safe(root, (Node *) scanjoin_target->exprs))
+               if (current_rel->partial_pathlist && scanjoin_target_parallel_safe)
                {
                        /* Apply the scan/join target to each partial path */
                        foreach(lc, current_rel->partial_pathlist)
@@ -1984,6 +2017,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                        current_rel = create_grouping_paths(root,
                                                                                                current_rel,
                                                                                                grouping_target,
+                                                                                               grouping_target_parallel_safe,
                                                                                                &agg_costs,
                                                                                                gset_data);
                        /* Fix things up if grouping_target contains SRFs */
@@ -2003,6 +2037,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                                                                                          current_rel,
                                                                                          grouping_target,
                                                                                          sort_input_target,
+                                                                                         sort_input_target_parallel_safe,
                                                                                          tlist,
                                                                                          wflists,
                                                                                          activeWindows);
@@ -2036,6 +2071,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
                current_rel = create_ordered_paths(root,
                                                                                   current_rel,
                                                                                   final_target,
+                                                                                  final_target_parallel_safe,
                                                                                   have_postponed_srfs ? -1.0 :
                                                                                   limit_tuples);
                /* Fix things up if final_target contains SRFs */
@@ -3623,6 +3659,7 @@ static RelOptInfo *
 create_grouping_paths(PlannerInfo *root,
                                          RelOptInfo *input_rel,
                                          PathTarget *target,
+                                         bool target_parallel_safe,
                                          const AggClauseCosts *agg_costs,
                                          grouping_sets_data *gd)
 {
@@ -3652,8 +3689,7 @@ create_grouping_paths(PlannerInfo *root,
         * target list and HAVING quals are parallel-safe.  The partially grouped
         * relation obeys the same rules.
         */
-       if (input_rel->consider_parallel &&
-               is_parallel_safe(root, (Node *) target->exprs) &&
+       if (input_rel->consider_parallel && target_parallel_safe &&
                is_parallel_safe(root, (Node *) parse->havingQual))
        {
                grouped_rel->consider_parallel = true;
@@ -4230,6 +4266,7 @@ create_window_paths(PlannerInfo *root,
                                        RelOptInfo *input_rel,
                                        PathTarget *input_target,
                                        PathTarget *output_target,
+                                       bool output_target_parallel_safe,
                                        List *tlist,
                                        WindowFuncLists *wflists,
                                        List *activeWindows)
@@ -4245,8 +4282,7 @@ create_window_paths(PlannerInfo *root,
         * can't be parallel-safe, either.  Otherwise, we need to examine the
         * target list and active windows for non-parallel-safe constructs.
         */
-       if (input_rel->consider_parallel &&
-               is_parallel_safe(root, (Node *) output_target->exprs) &&
+       if (input_rel->consider_parallel && output_target_parallel_safe &&
                is_parallel_safe(root, (Node *) activeWindows))
                window_rel->consider_parallel = true;
 
@@ -4621,6 +4657,7 @@ static RelOptInfo *
 create_ordered_paths(PlannerInfo *root,
                                         RelOptInfo *input_rel,
                                         PathTarget *target,
+                                        bool target_parallel_safe,
                                         double limit_tuples)
 {
        Path       *cheapest_input_path = input_rel->cheapest_total_path;
@@ -4635,8 +4672,7 @@ create_ordered_paths(PlannerInfo *root,
         * can't be parallel-safe, either.  Otherwise, it's parallel-safe if the
         * target list is parallel-safe.
         */
-       if (input_rel->consider_parallel &&
-               is_parallel_safe(root, (Node *) target->exprs))
+       if (input_rel->consider_parallel && target_parallel_safe)
                ordered_rel->consider_parallel = true;
 
        /*