(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:
Since I'm not 100% sure that that is the right way to go, I've been
rethinking how to fix this issue. Yet another idea I came up with to fix
this is to redesign the handling of the tlists for children in the
partitioning case. Currently, we build the reltarget for a child by
applying adjust_appendrel_attrs to the reltarget for its parent in
set_append_rel_size, which maps a wholerow Var referring to the parent rel
to a ConvertRowtypeExpr that translates a wholerow of the child rel into a
wholerow of the parent rel's rowtype. This works well for the
non-partitioned inheritance case, but makes complicated the code for
handling the partitioning case especially when planning partitionwise-joins.
And I think this would be the root cause of this issue.
Although true, this is not only about targetlist. Even the whole-row
expressions in the conditions, equivalence classes and other
planner/optimizer data structures are translated to
ConvertRowtypeExpr.
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.
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?
Best regards,
Etsuro Fujita
