On Thu, Jun 9, 2016 at 8:47 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Mon, Jun 6, 2016 at 6:07 AM, Amit Kapila <amit.kapil...@gmail.com>
> > 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
> > 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:



* 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 *)
  (Node *) rel->reltarget->exprs,

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:)

/* 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] -

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

Reply via email to