Re: [sqlsmith] Failed assertion in parallel worker (ExecInitSubPlan)

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andreas Seltenreich <seltenreich(at)gmx(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sqlsmith] Failed assertion in parallel worker (ExecInitSubPlan)
Date: 2016-06-09 13:37:16
Message-ID: CAA4eK1JOKS=6-vfd7=7tsUEu8RafLaMwLNcssFbRFUWgvQXH+A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 9, 2016 at 8:47 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Jun 6, 2016 at 6:07 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> > That seems doable, as for such rels we can only have Vars and
> > PlaceHolderVars in targetlist. Basically, whenever we are adding
> > PlaceHolderVars to a relation, just remember that information and use it
> > later. The patch doing the same is attached with this mail. Now still,
> > this won't cover the case of ChildRels for an Append Relation as for
that we
> > adjust target list separately in set_append_rel_size. I think we need
to
> > deal with it separately.
>
> This approach looks pretty good to me. Here's a revised version of
> your patch, with some renaming and other adjustments.
>

Your version looks good to me.

> I'm not sure
> exactly what you're referring to in set_append_rel_size,
>

The below code:

set_append_rel_size()
{
..

/*

* CE failed, so finish copying/modifying targetlist and join quals.
*
*
*NB: the resulting childrel->reltarget->exprs may contain arbitrary*
expressions, which otherwise would not occur in a rel's targetlist*.
..
*/

childrel->reltarget->exprs = (List *)
adjust_appendrel_attrs(root,
(Node *) rel->reltarget->exprs,
appinfo);

What I mean to say is if above code lead to some additional expressions in
childrel targetlist, then those can lead to some unsafe expressions in
child rel target list. This can cause some problem as we are blindly
considering that if parent rel is parallel safe, then child rel will also
be parallel safe (in below code:)

set_append_rel_size()
{
..
/* Copy consider_parallel flag from parent. */
childrel->consider_parallel = rel->consider_parallel;

> but I did add
> a line of code there that might be relevant.
>

I am not sure if that can address the above problem. I have posted my
analysis on the same in mail [1].

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

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-06-09 14:04:36 Re: Rename max_parallel_degree?
Previous Message Kyotaro HORIGUCHI 2016-06-09 12:55:58 [BUG] pg_basebackup from disconnected standby fails