pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Oddity with parallel safety test for scan/join target in grouping_planner
Date: Mon, 11 Mar 2019 12:56:21 +0900
Message-ID: <5C85DC65.2050409@lab.ntt.co.jp> (raw)
In-Reply-To: <24818.1552077419@sss.pgh.pa.us>
References: <5C75303E.8020303@lab.ntt.co.jp>
	<CA+TgmobmV=Zw2OEJKUorJe=e+_fCzsvYErR6tXxTjCzK45x9RQ@mail.gmail.com>
	<5C7908B9.5070407@lab.ntt.co.jp>
	<24818.1552077419@sss.pgh.pa.us>

(2019/03/09 5:36), Tom Lane wrote:
> Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp>  writes:
>> (2019/02/28 0:52), Robert Haas wrote:
>>> On Tue, Feb 26, 2019 at 7:26 AM Etsuro Fujita
>>> <fujita.etsuro@lab.ntt.co.jp>   wrote:
>>>> The parallel safety of the final scan/join target is determined from the
>>>> grouping target, not that target, which [ is wrong ]
>
>>> Your patch looks right to me.
>
>> I think it would be better for you to take this one.  Could you?
>
> I concur with Robert that this is a brown-paper-bag-grade bug in
> 960df2a97.  Please feel free to push (and don't forget to back-patch).

OK, will do.

> The only really interesting question is whether it's worth adding
> a regression test for.  I doubt it; the issue seems much too narrow.
> Usually the point of a regression test is to prevent re-introduction
> of the same/similar bug, but what class of bugs would you argue
> we'd be protecting against?

Plan degradation; without the fix, we would have this on data created by 
"pgbench -i -s 10 postgres", as shown in [1]:

postgres=# set parallel_setup_cost = 10;
postgres=# set parallel_tuple_cost = 0.001;

postgres=# 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} would be parallel 
safe, but in the plan the target is not parallelized across workers. 
The reason for that is because the parallel-safety of the target is 
assessed incorrectly using the grouping target {((aid + bid)), 
sum(abalance), random()}, which would not be parallel safe.  By the fix 
we would have this:

postgres=# explain verbose select aid+bid, sum(abalance), random() from 
pgbench_accounts group by aid+bid;
                                                 QUERY PLAN

-----------------------------------------------------------------------------------------------------------
  GroupAggregate  (cost=135944.68..158444.68 rows=1000000 width=20)
    Output: ((aid + bid)), sum(abalance), random()
    Group Key: ((pgbench_accounts.aid + pgbench_accounts.bid))
    ->  Sort  (cost=135944.68..138444.68 rows=1000000 width=8)
          Output: ((aid + bid)), abalance
          Sort Key: ((pgbench_accounts.aid + pgbench_accounts.bid))
          ->  Gather  (cost=10.00..22612.33 rows=1000000 width=8)
                Output: ((aid + bid)), abalance
                Workers Planned: 2
                ->  Parallel Seq Scan on public.pgbench_accounts 
(cost=0.00..21602.33 rows=416667 width=8)
-->                  Output: (aid + bid), abalance
(11 rows)

Note that the final scan/join target is parallelized in the plan.

This would only affect plan quality a little bit, so I don't think we 
need a regression test for this, either, but the fix might destabilize 
existing plan choices, so we should apply it to HEAD only?

Best regards,
Etsuro Fujita





view thread (12+ messages)  latest in thread

Message-ID: <5C85DC65.2050409@lab.ntt.co.jp>
Permalink:  ../5C85DC65.2050409@lab.ntt.co.jp/
Also on:    postgresql.org/message-id/5C85DC65.2050409@lab.ntt.co.jp

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: fujita.etsuro@lab.ntt.co.jp, tgl@sss.pgh.pa.us, robertmhaas@gmail.com
  Subject: Re: Oddity with parallel safety test for scan/join target in grouping_planner
  In-Reply-To: <5C85DC65.2050409@lab.ntt.co.jp>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox