Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.

(2018/05/18 16:33), Etsuro Fujita wrote:
> Other than pull_var_clause things,
> the updated version looks good to me, so I'll mark this as Ready for
> Committer.

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. 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.

Best regards,
Etsuro Fujita