Amit-san,
On Mon, Aug 5, 2019 at 2:36 PM Amit Langote <amitlangote09@gmail.com> wrote:
> 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! Will review the updated version of the FDW patch, at least.
Best regards,
Etsuro Fujita