Re: postgres_fdw: Oddity in pushing down inherited UPDATE/DELETE joins to remote servers

(2018/05/17 1:16), Robert Haas wrote:
> On Fri, May 11, 2018 at 8:46 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> I added an assertion test to make_modifytable to match that in
>> create_modifytable_path. Attached is an updated version of the patch.
>
> Committed.

Thanks you!

> Was it just good luck that this ever worked at all? I mean:
>
> - if (rti< root->simple_rel_array_size&&
> - root->simple_rel_array[rti] != NULL)
> + if (rti< subroot->simple_rel_array_size&&
> + subroot->simple_rel_array[rti] != NULL)
> {
> - RelOptInfo *resultRel = root->simple_rel_array[rti];
> + RelOptInfo *resultRel = subroot->simple_rel_array[rti];
>
> fdwroutine = resultRel->fdwroutine;
> }
> else
> {
> - RangeTblEntry *rte = planner_rt_fetch(rti, root);
> + RangeTblEntry *rte = planner_rt_fetch(rti, subroot);
>
> ...an RTI is only meaningful relative to a particular PlannerInfo's
> range table. It can't be right to just take an RTI for one
> PlannerInfo and look it up in some other PlannerInfo's range table.

I think that can be right; because inheritance_planner generates the
simple_rel_array and simple_rte_array for the parent PlannerInfo so that
it allows us to do that. This is the logic that that function creates
the simple_rel_array for the parent PlannerInfo:

/*
* We need to collect all the RelOptInfos from all child plans into
* the main PlannerInfo, since setrefs.c will need them. We
use the
* last child's simple_rel_array (previous ones are too short),
so we
* have to propagate forward the RelOptInfos that were already
built
* in previous children.
*/
Assert(subroot->simple_rel_array_size >= save_rel_array_size);
for (rti = 1; rti < save_rel_array_size; rti++)
{
RelOptInfo *brel = save_rel_array[rti];

if (brel)
subroot->simple_rel_array[rti] = brel;
}

Best regards,
Etsuro Fujita