Re: Parallel plans and "union all" subquery - Mailing list pgsql-hackers

From Luc Vlaming
Subject Re: Parallel plans and "union all" subquery
Date
Msg-id 7181dc48-c0ec-d409-5333-e4e135f4bf1e@swarm64.com
Whole thread Raw
In response to Re: Parallel plans and "union all" subquery  (Greg Nancarrow <gregn4422@gmail.com>)
Responses Re: Parallel plans and "union all" subquery  (Greg Nancarrow <gregn4422@gmail.com>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Have collation versioning to ignore hash and similar AM
Next
From: "osumi.takamichi@fujitsu.com"
Date:
Subject: Stronger safeguard for archive recovery not to miss data