Thread: Parallel plans and "union all" subquery

On Sun, Nov 22, 2020 at 11:51 PM Phil Florent <philflorent@hotmail.com> wrote:
>
>
> Hi,
>
>
> I have a question about parallel plans. I also posted it on the general list but perhaps it's a question for hackers.
Hereis my test case :
 
>
>
> explain
> select count(*)
> from (select
> n1
> from drop_me
> union all
> values(1)) ua;
>
>
> QUERY PLAN
> --------------------------------------------------------------------------------
> Aggregate (cost=2934739.24..2934739.25 rows=1 width=8)
> -> Append (cost=0.00..2059737.83 rows=70000113 width=32)
> -> Seq Scan on drop_me (cost=0.00..1009736.12 rows=70000112 width=6)
> -> Subquery Scan on "*SELECT* 2" (cost=0.00..0.02 rows=1 width=32)
> -> Result (cost=0.00..0.01 rows=1 width=4)
> JIT:
> Functions: 4
> Options: Inlining true, Optimization true, Expressions true, Deforming true
>
>
> No parallel plan, 2s6
>
>
> I read the documentation but I don't get the reason of the "noparallel" seq scan of drop_me in the last case ?
>

Without debugging this, it looks to me that the UNION type resolution
isn't working as well as it possibly could in this case, for the
generation of a parallel plan. I found that with a minor tweak to your
SQL, either for the table creation or query, it will produce a
parallel plan.

Noting that currently you're creating the drop_me table with a
"numeric" column, you can either:

(1) Change the table creation

FROM:
    create unlogged table drop_me as select generate_series(1,7e7) n1;
TO:
    create unlogged table drop_me as select generate_series(1,7e7)::int n1;


OR


(2) Change the query

FROM:
    explain
    select count(*)
    from (select
    n1
    from drop_me
    union all
    values(1)) ua;

TO:

    explain
    select count(*)
    from (select
    n1
    from drop_me
    union all
    values(1::numeric)) ua;


                                            QUERY PLAN
----------------------------------------------------------------------------------------------------
 Finalize Aggregate  (cost=821152.71..821152.72 rows=1 width=8)
   ->  Gather  (cost=821152.50..821152.71 rows=2 width=8)
         Workers Planned: 2
         ->  Partial Aggregate  (cost=820152.50..820152.51 rows=1 width=8)
               ->  Parallel Append  (cost=0.00..747235.71 rows=29166714 width=0)
                     ->  Result  (cost=0.00..0.01 rows=1 width=0)
                     ->  Parallel Seq Scan on drop_me
(cost=0.00..601402.13 rows=29166713 width=0)
(7 rows)


Regards,
Greg Nancarrow
Fujitsu Australia

On 23-11-2020 13:17, Phil Florent wrote:
> Hi Greg,
> 
> The implicit conversion was the cause of the non parallel plan, thanks 
> for the explanation and the workarounds. It can cause a huge difference 
> in terms of performance, I will give the information to our developers.
> 
> Regards,
> 
> Phil
> 
> 
> 
> ------------------------------------------------------------------------
> *De :* Greg Nancarrow <gregn4422@gmail.com>
> *Envoyé :* lundi 23 novembre 2020 06:04
> *À :* Phil Florent <philflorent@hotmail.com>
> *Cc :* pgsql-hackers@lists.postgresql.org 
> <pgsql-hackers@lists.postgresql.org>
> *Objet :* Re: Parallel plans and "union all" subquery
> On Sun, Nov 22, 2020 at 11:51 PM Phil Florent <philflorent@hotmail.com> 
> wrote:
>>
>>
>> Hi,
>>
>>
>> I have a question about parallel plans. I also posted it on the general list but perhaps it's a question for
hackers.Here is my test case :
 
>>
>>
>> explain
>> select count(*)
>> from (select
>> n1
>> from drop_me
>> union all
>> values(1)) ua;
>>
>>
>> QUERY PLAN
>> --------------------------------------------------------------------------------
>> Aggregate (cost=2934739.24..2934739.25 rows=1 width=8)
>> -> Append (cost=0.00..2059737.83 rows=70000113 width=32)
>> -> Seq Scan on drop_me (cost=0.00..1009736.12 rows=70000112 width=6)
>> -> Subquery Scan on "*SELECT* 2" (cost=0.00..0.02 rows=1 width=32)
>> -> Result (cost=0.00..0.01 rows=1 width=4)
>> JIT:
>> Functions: 4
>> Options: Inlining true, Optimization true, Expressions true, Deforming true
>>
>>
>> No parallel plan, 2s6
>>
>>
>> I read the documentation but I don't get the reason of the "noparallel" seq scan of drop_me in the last case ?
>>
> 
> Without debugging this, it looks to me that the UNION type resolution
> isn't working as well as it possibly could in this case, for the
> generation of a parallel plan. I found that with a minor tweak to your
> SQL, either for the table creation or query, it will produce a
> parallel plan.
> 
> Noting that currently you're creating the drop_me table with a
> "numeric" column, you can either:
> 
> (1) Change the table creation
> 
> FROM:
>      create unlogged table drop_me as select generate_series(1,7e7) n1;
> TO:
>      create unlogged table drop_me as select generate_series(1,7e7)::int n1;
> 
> 
> OR
> 
> 
> (2) Change the query
> 
> FROM:
>      explain
>      select count(*)
>      from (select
>      n1
>      from drop_me
>      union all
>      values(1)) ua;
> 
> TO:
> 
>      explain
>      select count(*)
>      from (select
>      n1
>      from drop_me
>      union all
>      values(1::numeric)) ua;
> 
> 
>                                              QUERY PLAN
> ----------------------------------------------------------------------------------------------------
>   Finalize Aggregate  (cost=821152.71..821152.72 rows=1 width=8)
>     ->  Gather  (cost=821152.50..821152.71 rows=2 width=8)
>           Workers Planned: 2
>           ->  Partial Aggregate  (cost=820152.50..820152.51 rows=1 width=8)
>                 ->  Parallel Append  (cost=0.00..747235.71 rows=29166714 
> width=0)
>                       ->  Result  (cost=0.00..0.01 rows=1 width=0)
>                       ->  Parallel Seq Scan on drop_me
> (cost=0.00..601402.13 rows=29166713 width=0)
> (7 rows)
> 
> 
> Regards,
> Greg Nancarrow
> Fujitsu Australia

Hi,

For this problem there is a patch I created, which is registered under 
https://commitfest.postgresql.org/30/2787/ that should fix this without 
any workarounds. Maybe someone can take a look at it?

Regards,
Luc
Swarm64

On Tue, Nov 24, 2020 at 2:34 AM Luc Vlaming <luc@swarm64.com> wrote:
>
> Hi,
>
> For this problem there is a patch I created, which is registered under
> https://commitfest.postgresql.org/30/2787/ that should fix this without
> any workarounds. Maybe someone can take a look at it?
>

I tried your patch with the latest PG source code (24/11), but
unfortunately a non-parallel plan was still produced in this case.

test=# explain
select count(*)
from (select
n1
from drop_me
union all
values(1)) ua;
                                   QUERY PLAN
--------------------------------------------------------------------------------
 Aggregate  (cost=1889383.54..1889383.55 rows=1 width=8)
   ->  Append  (cost=0.00..1362834.03 rows=42123961 width=32)
         ->  Seq Scan on drop_me  (cost=0.00..730974.60 rows=42123960 width=32)
         ->  Subquery Scan on "*SELECT* 2"  (cost=0.00..0.02 rows=1 width=32)
               ->  Result  (cost=0.00..0.01 rows=1 width=4)
(5 rows)


That's not to say your patch doesn't have merit - but maybe just not a
fix for this particular case.

As before, if the SQL is tweaked to align the types for the UNION, you
get a parallel plan:

test=# explain
select count(*)
from (select
n1
from drop_me
union all
values(1::numeric)) ua;
                                             QUERY PLAN
----------------------------------------------------------------------------------------------------
 Finalize Aggregate  (cost=821152.71..821152.72 rows=1 width=8)
   ->  Gather  (cost=821152.50..821152.71 rows=2 width=8)
         Workers Planned: 2
         ->  Partial Aggregate  (cost=820152.50..820152.51 rows=1 width=8)
               ->  Parallel Append  (cost=0.00..747235.71 rows=29166714 width=0)
                     ->  Result  (cost=0.00..0.01 rows=1 width=0)
                     ->  Parallel Seq Scan on drop_me
(cost=0.00..601402.13 rows=29166713 width=0)
(7 rows)


Regards,
Greg Nancarrow
Fujitsu Australia

On 24-11-2020 01:44, Greg Nancarrow wrote:
> On Tue, Nov 24, 2020 at 2:34 AM Luc Vlaming <luc@swarm64.com> wrote:
>>
>> Hi,
>>
>> For this problem there is a patch I created, which is registered under
>> https://commitfest.postgresql.org/30/2787/ that should fix this without
>> any workarounds. Maybe someone can take a look at it?
>>
> 
> I tried your patch with the latest PG source code (24/11), but
> unfortunately a non-parallel plan was still produced in this case.
> 
> test=# explain
> select count(*)
> from (select
> n1
> from drop_me
> union all
> values(1)) ua;
>                                     QUERY PLAN
> --------------------------------------------------------------------------------
>   Aggregate  (cost=1889383.54..1889383.55 rows=1 width=8)
>     ->  Append  (cost=0.00..1362834.03 rows=42123961 width=32)
>           ->  Seq Scan on drop_me  (cost=0.00..730974.60 rows=42123960 width=32)
>           ->  Subquery Scan on "*SELECT* 2"  (cost=0.00..0.02 rows=1 width=32)
>                 ->  Result  (cost=0.00..0.01 rows=1 width=4)
> (5 rows)
> 
> 
> That's not to say your patch doesn't have merit - but maybe just not a
> fix for this particular case.
> 
> As before, if the SQL is tweaked to align the types for the UNION, you
> get a parallel plan:
> 
> test=# explain
> select count(*)
> from (select
> n1
> from drop_me
> union all
> values(1::numeric)) ua;
>                                               QUERY PLAN
> ----------------------------------------------------------------------------------------------------
>   Finalize Aggregate  (cost=821152.71..821152.72 rows=1 width=8)
>     ->  Gather  (cost=821152.50..821152.71 rows=2 width=8)
>           Workers Planned: 2
>           ->  Partial Aggregate  (cost=820152.50..820152.51 rows=1 width=8)
>                 ->  Parallel Append  (cost=0.00..747235.71 rows=29166714 width=0)
>                       ->  Result  (cost=0.00..0.01 rows=1 width=0)
>                       ->  Parallel Seq Scan on drop_me
> (cost=0.00..601402.13 rows=29166713 width=0)
> (7 rows)
> 
> 
> Regards,
> Greg Nancarrow
> Fujitsu Australia
> 

Hi,

You're completely right, sorry for my error. I was too quick on assuming 
my patch would work for this specific case too; I should have tested 
that before replying. It looked very similar but turns out to not work 
because of the upper rel not being considered parallel.

I would like to extend my patch to support this, or create a second 
patch. This would however be significantly more involved because it 
would require that we (always?) consider two paths whenever we process a 
subquery: the best parallel plan and the best serial plan. Before I 
emback on such a journey I would like some input on whether this would 
be a very bad idea. Thoughts?

Regards,
Luc
Swarm64

On 25-11-2020 14:54, Greg Nancarrow wrote:
> 
> 
> On Wed, Nov 25, 2020 at 6:43 PM Luc Vlaming <luc@swarm64.com 
> <mailto:luc@swarm64.com>> wrote:
>  >
>  >
>  > You're completely right, sorry for my error. I was too quick on assuming
>  > my patch would work for this specific case too; I should have tested
>  > that before replying. It looked very similar but turns out to not work
>  > because of the upper rel not being considered parallel.
>  >
>  > I would like to extend my patch to support this, or create a second
>  > patch. This would however be significantly more involved because it
>  > would require that we (always?) consider two paths whenever we process a
>  > subquery: the best parallel plan and the best serial plan. Before I
>  > emback on such a journey I would like some input on whether this would
>  > be a very bad idea. Thoughts?
>  >
> 
> Hi,
> 
> I must admit, your intended approach isn't what immediately came to mind 
> when I saw this issue. Have you analyzed and debugged this to know 
> exactly what is going on?
> I haven't had time to debug this and see exactly where the code paths 
> diverge for the use of "values(1)" verses "values(1::numeric)" in this 
> case, but that would be one of the first steps.
> 
> What I wondered (and I may well be wrong) was how come the documented 
> type resolution algorithm 
> (https://www.postgresql.org/docs/13/typeconv-union-case.html 
> <https://www.postgresql.org/docs/13/typeconv-union-case.html>) doesn't 
> seem to be working quite right here, at least to the point of creating 
> the same/similar parse tree as when I change "values(1)" to 
> "values(1::numeric)" or even just "values(1.)"? So shouldn't then  the 
> use of "values(1)" in this case (a constant, convertible to numeric - 
> the preferred type ) result in the same (parallel) plan as when 
> "values(1::numeric)" is used? Perhaps this isn't happening because the 
> code is treating these as generalised expressions when their types 
> aren't the same, and this then affects parsing/planning?
> My natural thought was that there seems to be a minor issue in the code, 
> which should be reasonably easy to fix, at least for this fairly simple 
> case.
> 
> However, I claim no expertise in the area of parser/analyzer/planner, I 
> only know certain areas of that code, but enough to appreciate it is 
> complex and intricate, and easily broken.
> Perhaps one of the major contributors to this area of the code, who 
> probably know this code very well, like maybe Tom Lane or Robert Haas 
> (to name two) might like to comment on whether what we're looking at is 
> indeed a bug/deficiency and worth fixing, and whether Luc is correct in 
> his expressed approach on what would be required to fix it?
> 
> Regards,
> Greg Nancarrow
> Fujitsu Australia

So from what I recall from building the patch is that the difference is 
that when all types are identical, then flatten_simple_union_all simply 
flattens all union-all operations into an append relation.
If you don't have identical types then the situation has to be handled 
by the code in prepunion.c which doesn't always keep a parallel path 
around. The patch I had posted fixes this for a relatively simple issue 
and not the case described here.
If interesting I can make a draft of what this would look like if this 
makes it easier to discuss?

Regards,
Luc
Swarm64

On Thu, Nov 26, 2020 at 6:11 PM Luc Vlaming <luc@swarm64.com> wrote:
>
> If interesting I can make a draft of what this would look like if this
> makes it easier to discuss?
>

Sure, that would help clarify it.

I did debug this a bit, but it seems my gut feeling was wrong, even
though it knows a type coercion is required and can be done, the
parse/analyze code doesn't actually modify the nodes in place "for
fear of changing the semantics", so when the types don't exactly match
it's all left up to the planner, but for this parse tree it fails to
produce a parallel plan.

Regards,
Greg Nancarrow
Fujitsu Australia

On 27-11-2020 04:14, Greg Nancarrow wrote:
> On Thu, Nov 26, 2020 at 6:11 PM Luc Vlaming <luc@swarm64.com> wrote:
>>
>> If interesting I can make a draft of what this would look like if this
>> makes it easier to discuss?
>>
> 
> Sure, that would help clarify it.
Okay. I will try to build an example but this will take a few weeks as 
vacations and such are coming up too.

> 
> I did debug this a bit, but it seems my gut feeling was wrong, even
> though it knows a type coercion is required and can be done, the
> parse/analyze code doesn't actually modify the nodes in place "for
> fear of changing the semantics", so when the types don't exactly match
> it's all left up to the planner, but for this parse tree it fails to
> produce a parallel plan.
> 

Yes. However I think here also lies an opportunity, because to me it 
seems much more appealing to have the planner being able to deal 
correctly with all the situations rather than having things like 
flatten_simple_union_all() that provide a solution for the ideal case.

> Regards,
> Greg Nancarrow
> Fujitsu Australia
> 

Regards,
Luc
Swarm64