Re: POC: postgres_fdw insert batching

On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> On 2/5/21 3:52 AM, Amit Langote wrote:
> > Tsunakwa-san,
> >
> > On Mon, Jan 25, 2021 at 1:21 PM tsunakawa(dot)takay(at)fujitsu(dot)com
> > <tsunakawa(dot)takay(at)fujitsu(dot)com> wrote:
> >> From: Amit Langote <amitlangote09(at)gmail(dot)com>
> >>> Yes, it can be simplified by using a local join to prevent the update of the foreign
> >>> partition from being pushed to the remote server, for which my example in the
> >>> previous email used a local trigger. Note that the update of the foreign
> >>> partition to be done locally is a prerequisite for this bug to occur.
> >>
> >> Thank you, I was aware that UPDATE calls ExecInsert() but forgot about it partway. Good catch (and my bad miss.)
> >
> > It appears I had missed your reply, sorry.
> >
> >> + PgFdwModifyState *fmstate = resultRelInfo->ri_FdwState ?
> >> + (PgFdwModifyState *) resultRelInfo->ri_FdwState :
> >> + NULL;
> >>
> >> This can be written as:
> >>
> >> + PgFdwModifyState *fmstate = (PgFdwModifyState *) resultRelInfo->ri_FdwState;
> >
> > Facepalm, yes.
> >
> > Patch updated. Thanks for the review.
> >
>
> Thanks for the patch, it seems fine to me.

Thanks for checking.

> I wonder it the commit
> message needs some tweaks, though. At the moment it says:
>
> Prevent FDW insert batching during cross-partition updates
>
> but what the patch seems to be doing is simply initializing the info
> only for CMD_INSERT operations. Which does the trick, but it affects
> everything, i.e. all updates, no? Not just cross-partition updates.

You're right. Please check the message in the updated patch.

--
Amit Langote
EDB: http://www.enterprisedb.com