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

Fujita-san,

On 2018/05/17 21:51, Etsuro Fujita wrote:
> (2018/05/17 14:19), Amit Langote wrote:
>> Looking at this for a bit, I wondered if this crash wouldn't have occurred
>> if the "propagation" had also considered join relations in addition to
>> simple relations.  For example, if I changed inheritance_planner like the
>> attached (not proposing that we consider committing it), reported crash
>> doesn't occur.  The fact that it's not currently that way means that
>> somebody thought that there is no point in keeping all of those joinrels
>> around until plan creation time.
>
> One reason for that would be that we use the per-child PlannerInfo, not
> the parent one, at plan creation time.  Here is the code in
> create_modifytable_plan:
>
>         /*
>          * In an inherited UPDATE/DELETE, reference the per-child modified
>          * subroot while creating Plans from Paths for the child rel. This is
>          * a kluge, but otherwise it's too hard to ensure that Plan creation
>          * functions (particularly in FDWs) don't depend on the contents of
>          * "root" matching what they saw at Path creation time.  The main
>          * downside is that creation functions for Plans that might appear
>          * below a ModifyTable cannot expect to modify the contents of "root"
>          * and have it "stick" for subsequent processing such as setrefs.c.
>          * That's not great, but it seems better than the alternative.
>          */
>         subplan = create_plan_recurse(subroot, subpath, CP_EXACT_TLIST);
>
> So, we don't need to accumulate the joinrel lists for child relations into
> a single list and store that list into the parent PlannerInfo in
> inheritance_planner, as in the patch you proposed.  I think the change by
> the commit is based on the same idea as that.

OK, thanks for explaining. I am not questioning what's already committed
as a fix for this issue, nor am I proposing the patch that I posted
earlier to be considered for committing. I was just idly wondering why
PlanDirectModifyTable looks for joinrel RelOptInfos in the plan creation
phase if they're not guaranteed to exist, especially if no other plan
creation functions do. For instance, postgres_fdw's
postgresPlanDirectModify appears to be the only function that's invoked in
the plan creation phase that's doing a find_join_rel(). Maybe, the rule
that joinrels shouldn't be accessed this late doesn't exist and hence we
shouldn't be worried.

Thanks,
Amit