(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
