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

Hi Amit,

(2018/05/11 16:12), Amit Langote wrote:
> On 2018/05/10 21:41, Etsuro Fujita wrote:
>> I think the reason for that is: in that case we try to find the target
>> foreign-join RelOptInfo using find_join_rel in postgresPlanDirectModify,
>> but can't find it, because the given root is the *parent* root and
>> doesn't have join RelOptInfos with it.  To fix this, I'd like to propose
>> to modify make_modifytable so that in case of an inherited
>> UPDATE/DELETE, it passes to PlanDirectModify the per-child modified
>> subroot, not the parent root, for the FDW to get the foreign-join
>> RelOptInfo from the given root in PlanDirectModify.  I think that that
>> would be more consistent with the non-inherited UPDATE/DELETE case in
>> that the FDW can look at any join RelOptInfos in the given root in
>> PlanDirectModify, which I think would be a good thing because some FDWs
>> might need to do that for some reason.  For the same reason, I'd also
>> like to propose to pass to PlanForeignModify the per-child modified
>> subroot, not the parent root, as for PlanDirectModify.  Attached is a
>> proposed patch for that.  I'll add this to the open items list for PG11.
> 
> So IIUC, we must pass the per-child PlannerInfo here, because that's what
> would have been used for the planning join between the child (ft1 in your
> example) and the other table (ft2 in your example).  So that's where the
> RelOptInfo's corresponding to planning for the child, including that for
> the pushed-down join, would be stored.

Yeah, I think so too.

> Anyway, the patch and tests it adds look good.

Thanks for the review!

I added an assertion test to make_modifytable to match that in
create_modifytable_path.  Attached is an updated version of the patch.

Best regards,
Etsuro Fujita