Amit-san, On Thu, Dec 8, 2022 at 5:00 PM Amit Langote <amitlangot...@gmail.com> wrote: > On Tue, Dec 6, 2022 at 6:47 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: > > * In postgresGetForeignModifyBatchSize(): > > > > /* > > - * Should never get called when the insert is being performed as part > > of a > > - * row movement operation. > > + * Use the auxiliary state if any; see postgresBeginForeignInsert() for > > + * details on what it represents. > > */ > > - Assert(fmstate == NULL || fmstate->aux_fmstate == NULL); > > + if (fmstate != NULL && fmstate->aux_fmstate != NULL) > > + fmstate = fmstate->aux_fmstate; > > > > I might be missing something, but I think we should leave the Assert > > as-is, because we still disallow to move rows to another foreign-table > > partition that is also an UPDATE subplan result relation, which means > > that any fmstate should have fmstate->aux_fmstate=NULL. > > Hmm, yes. I forgot that 86dc90056df effectively disabled *all* > attempts of inserting into foreign partitions that are also UPDATE > target relations, so you are correct that fmstate->aux_fmstate would > never be set when entering this function. > > That means this functionality is only useful for foreign partitions > that are not also being updated by the original UPDATE.
Yeah, I think so too. > I've reinstated the Assert, removed the if block as it's useless, and > updated the comment a bit to clarify the restriction a bit. Looks good to me. > > * Also in that function: > > > > - if (fmstate) > > + if (fmstate != NULL) > > > > This is correct, but I would vote for leaving that as-is, to make > > back-patching easy. > > Removed this hunk. Thanks! > Updated patch attached. Thanks for the patch! I will review the patch a bit more, but I think it would be committable. Best regards, Etsuro Fujita
