(2018/06/14 22:37), Ashutosh Bapat wrote:
On Mon, Jun 11, 2018 at 4:05 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
(2018/06/07 19:42), Ashutosh Bapat wrote:
On Wed, Jun 6, 2018 at 5:00 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
Yeah, but I mean we modify set_append_rel_size so that we only map a parent
wholerow Var in the parent tlist to a child wholerow Var in the child's
tlist; parent wholerow Vars in the parent's other expressions such as
conditions are transformed into CREs as-is.
What happens to a PlaceHolderVar containing whole-row reference when
that appears in a condition and/or targetlist.
Whole-row Vars contained in such PHV's expressions will be mapped into
ConvertRowtypeExprs with adjust_appendrel_attrs.
I don't think the
tlists for the children need to have their wholerows transformed into the
corresponding ConvertRowtypeExprs *at this point*, so what I'd like to
propose is to 1) map a parent wholerow Var simply to a child wholerow
Var,
instead (otherwise, the same as adjust_appendrel_attrs), when building
the
reltarget for a child in set_append_rel_size, 2) build the tlists for
child
joins using those children's wholerow Vars at path creation time, and 3)
adjust those wholerow Vars in the tlists for subpaths in the chosen
AppendPath so that those are transformed into the corresponding
ConvertRowtypeExprs, at plan creation time (ie, in
create_append_plan/create_merge_append_plan). IIUC, this would not
require
any changes to pull_var_clause as proposed by the patch. This would not
require any changes to postgres_fdw as proposed by the patch, either. In
addition, this would make unnecessary the code added to setrefs.c by the
partitionwise-join patch. Maybe I'm missing something though.
Not translating whole-row expressions to ConvertRowtypeExpr before
creating paths can lead to a number of anomalies. For example,
1. if there's an index on the whole-row expression of child,
translating parent's whole-row expression to child's whole-row
expression as is will lead to using that index, when in reality the it
can not be used since the condition/ORDER BY clause (which originally
referred the parent's whole-row expression) require the child's
whole-row reassembled as parent's whole-row.
2. Constraints on child'd whole-row expression, will be used to prune
a child when they can not be used since the original condition was on
parent' whole-row expression and not that of the child.
3. Equivalence classes will think that a child whole-row expression
(without ConvertRowtypeExpr) is equivalent to an expression which is
part of the same equivalence class as the parent' whole-row
expression.
Is that still possible when we only map a parent wholerow Var in the
parent's tlist to a child wholerow Var in the child's tlist?
Yes. 1 will affect the choice of index-only scans. We use pathkey
comparisons at a number of places so tlist going out of sync with
equivalence classes can affect a number of places.
Sorry, I'm still not sure if #1 is really a problem. Could you show me
an example for that?
build_tlist_to_deparse() is used to deparse targetlist at the time of
creating paths,as well as during the planning. According to your
proposal we will build different tlists during path creation and plan
creation. That doesn't look good.
Maybe my explanation was not enough, but my proposal will guarantee that
the FDW can build the same tlist at path creation time and plan creation
time because the RelOptInfo's reltarget from which the tlist is created
doesn't change at all.
Apart from that your proposal of
changing a child's targetlist at the time of plan creation to use CRE
doesn't help since the original problem as described in [1] happens at
the time of creating plans as described in [1].
I think my proposal will address the original issue. Actually, I've
created a patch implementing that proposal. That's still WIP, but it
works well even for these cases (and makes code much more simple, as
mentioned above!) But I think that patch needs more work, so I'm
planning to post it next week.
Best regards,
Etsuro Fujita
