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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw: Oddity in pushing down inherited UPDATE/DELETE joins to remote servers
Date: 2018-05-11 12:46:05
Message-ID: 5AF5908D.2060404@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
make_modifytable-2.patch text/x-diff 10.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2018-05-11 12:48:49 Re: postgres_fdw: Oddity in pushing down inherited UPDATE/DELETE joins to remote servers
Previous Message Vladimir Sitnikov 2018-05-11 12:33:05 Re: Having query cache in core