Thread: Append's first_partial_plan

Append's first_partial_plan

From
Alvaro Herrera
Date:
David Rowley wrote in https://postgr.es/m/CAKJS1f8o2Yd=rOP=Et3A0FWgF+gSAOkFSU6eNhnGzTPV7nN8sQ@mail.gmail.com

> I've made another pass over the nodeAppend.c code and I'm unable to
> see what might cause this, although I did discover a bug where
> first_partial_plan is not set taking into account that some subplans
> may have been pruned away during executor init. The only thing I think
> this would cause is for parallel workers to not properly help out with
> some partial plans if some earlier subplans were pruned. I can see no
> reason for this to have caused this particular issue since the
> first_partial_plan would be 0 with and without the attached fix.

While looking at this patch I became curious as to why do we even have
first_partial_plan in the first place; it seems to require some strange
contortions in the code.  Wouldn't it be simpler to have two lists, one
for non-partial and another for partial paths?  I went to check the
original discussion, and this design was indeed considered [1] -- but
the idea was discarded because using the list index would lead to
simpler code.  However, now that we have pruning it seems to me that
using the index isn't simpler anymore.  Should we revisit this now?

[1] https://www.postgresql.org/message-id/CA%2BTgmoZrjAB0bPzbtKgjP2uAP_6XAyuZenU54QuM7XGE_k2Q1g%40mail.gmail.com

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Append's first_partial_plan

From
David Rowley
Date:
On 18 April 2018 at 07:52, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> While looking at this patch I became curious as to why do we even have
> first_partial_plan in the first place; it seems to require some strange
> contortions in the code.  Wouldn't it be simpler to have two lists, one
> for non-partial and another for partial paths?  I went to check the
> original discussion, and this design was indeed considered [1] -- but
> the idea was discarded because using the list index would lead to
> simpler code.  However, now that we have pruning it seems to me that
> using the index isn't simpler anymore.  Should we revisit this now?
>
> [1] https://www.postgresql.org/message-id/CA%2BTgmoZrjAB0bPzbtKgjP2uAP_6XAyuZenU54QuM7XGE_k2Q1g%40mail.gmail.com

I don't think having two Lists and/or two AppendState arrays would
make the pruning code anymore simple. All the pruning code in
execPartition.c would need to determine the index within the partial
or non-partial subnode array, and also communicate which array it
means.  That code did take me a while to get right and be readable
too, I don't really want to have to change it again. I really don't
think it would look quite as simple as it does today either, so -1
from me for changing this.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


Re: Append's first_partial_plan

From
Alvaro Herrera
Date:
David Rowley wrote:
> On 18 April 2018 at 07:52, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > While looking at this patch I became curious as to why do we even have
> > first_partial_plan in the first place; it seems to require some strange
> > contortions in the code.  Wouldn't it be simpler to have two lists, one
> > for non-partial and another for partial paths?  I went to check the
> > original discussion, and this design was indeed considered [1] -- but
> > the idea was discarded because using the list index would lead to
> > simpler code.  However, now that we have pruning it seems to me that
> > using the index isn't simpler anymore.  Should we revisit this now?
>
> I don't think having two Lists and/or two AppendState arrays would
> make the pruning code anymore simple. All the pruning code in
> execPartition.c would need to determine the index within the partial
> or non-partial subnode array, and also communicate which array it
> means.  That code did take me a while to get right and be readable
> too, I don't really want to have to change it again. I really don't
> think it would look quite as simple as it does today either, so -1
> from me for changing this.

Got it.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services