Fujita-san,
Thanks for the quick follow up.
On Mon, Aug 5, 2019 at 2:31 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Mon, Aug 5, 2019 at 1:31 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Sun, Aug 4, 2019 at 4:45 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > > On Sun, Aug 4, 2019 at 3:03 AM Andres Freund <andres@anarazel.de> wrote:
> > > > On 2019-08-03 13:48:01 -0400, Tom Lane wrote:
> > > > > If those are the choices, adding a parameter is clearly the preferable
> > > > > solution, because it makes the API breakage obvious at compile.
> > > >
> > > > Right. I think it's a *bit* less clear in this case because we'd also
> > > > remove the field that such FDWs with direct modify support would use
> > > > now (EState.es_result_relation_info).
> > > >
> > > > But I think it's also just plainly a better API to use the
> > > > parameter. Even if, in contrast to the BeginDirectModify at hand,
> > > > BeginForeignModify didn't already accept it. Requiring a function call to
> > > > gather information that just about every realistic implementation is
> > > > going to need doesn't make sense.
> > >
> > > Agreed.
> >
> > So, is it correct to think that the consensus is to add a parameter to
> > BeginDirectModify()?
>
> I think so.
>
> > Also, avoid changing where BeginDirectModify() is called from, like my
> > patch did, only to have easy access to the ResultRelInfo to pass. We
> > can do that by by augmenting ForeignScan node to add the information
> > needed to fetch the ResultRelInfo efficiently from
> > ExecInitForeignScan() itself.
>
> I think so.
>
> > That information is the ordinal
> > position of a given result relation in PlannedStmt.resultRelations,
> > not the RT index as we were discussing.
>
> Yeah, that would be what Andres is proposing, which I think is much
> better than what I proposed using the RT index.
>
> Could you update your patch?
OK, I will do that. I'll reply with the updated patches to an
upthread email of Andres' [1], where he also comments on the other
patches.
Thanks,
Amit
[1] https://www.postgresql.org/message-id/20190802180138.64zcircokw2upaho%40alap3.anarazel.de