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

(2018/05/17 1:16), Robert Haas wrote:
> On Fri, May 11, 2018 at 8:46 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.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