Message-ID: <5C75303E.8020303@lab.ntt.co.jp>
Date: Tue, 26 Feb 2019 21:25:34 +0900
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:11.0) Gecko/20120327 Thunderbird/11.0.1
MIME-Version: 1.0
Subject: Oddity with parallel safety test for scan/join target in
 grouping_planner
Content-Type: multipart/mixed;
 boundary="------------040108010503000902030402"
To: Pg Hackers <pgsql-hackers@postgresql.org>
Precedence: bulk

This is a multi-part message in MIME format.
--------------040108010503000902030402
Content-Type: text/plain; charset=ISO-2022-JP
Content-Transfer-Encoding: 7bit

Hi hackers,

While working on [1], I noticed $Subject, that is:

        /*
         * If we have grouping or aggregation to do, the topmost scan/join
         * plan node must emit what the grouping step wants; otherwise, it
         * should emit grouping_target.
         */
        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;
        }

The parallel safety of the final scan/join target is determined from the
grouping target, not that target, which would cause to generate inferior
parallel plans as shown below:

pgbench=# explain verbose select aid+bid, sum(abalance), random() from
pgbench_accounts group by aid+bid;
                                                 QUERY PLAN
------------------------------------------------------------------------------------------------------------
 GroupAggregate  (cost=137403.01..159903.01 rows=1000000 width=20)
   Output: ((aid + bid)), sum(abalance), random()
   Group Key: ((pgbench_accounts.aid + pgbench_accounts.bid))
   ->  Sort  (cost=137403.01..139903.01 rows=1000000 width=8)
         Output: ((aid + bid)), abalance
         Sort Key: ((pgbench_accounts.aid + pgbench_accounts.bid))
         ->  Gather  (cost=10.00..24070.67 rows=1000000 width=8)
               Output: (aid + bid), abalance
               Workers Planned: 2
               ->  Parallel Seq Scan on public.pgbench_accounts
(cost=0.00..20560.67 rows=416667 width=12)
                     Output: aid, bid, abalance
(11 rows)

The final scan/join target {(aid + bid), abalance} is definitely
parallel safe, but the target evaluation isn't parallelized across
workers, which is not good.  Attached is a patch for fixing this.

Best regards,
Etsuro Fujita

[1] https://commitfest.postgresql.org/22/1950/

--------------040108010503000902030402
Content-Type: text/x-patch;
 name="target_parallel_safe.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="target_parallel_safe.patch"

*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
***************
*** 1989,1995 **** grouping_planner(PlannerInfo *root, bool inheritance_update,
  		{
  			scanjoin_target = make_group_input_target(root, final_target);
  			scanjoin_target_parallel_safe =
! 				is_parallel_safe(root, (Node *) grouping_target->exprs);
  		}
  		else
  		{
--- 1989,1995 ----
  		{
  			scanjoin_target = make_group_input_target(root, final_target);
  			scanjoin_target_parallel_safe =
! 				is_parallel_safe(root, (Node *) scanjoin_target->exprs);
  		}
  		else
  		{

--------------040108010503000902030402--