On 25-11-2020 14:54, Greg Nancarrow wrote:
On Wed, Nov 25, 2020 at 6:43 PM Luc Vlaming <l...@swarm64.com
<mailto:l...@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
