Fujita-san, Thanks for the quick follow up.
On Mon, Aug 5, 2019 at 2:31 PM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: > On Mon, Aug 5, 2019 at 1:31 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Sun, Aug 4, 2019 at 4:45 AM Etsuro Fujita <etsuro.fuj...@gmail.com> > > wrote: > > > On Sun, Aug 4, 2019 at 3:03 AM Andres Freund <and...@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
