Hi, On Wed, Aug 7, 2019 at 11:47 AM Amit Langote <amitlangot...@gmail.com> wrote: > On Wed, Aug 7, 2019 at 11:30 AM Etsuro Fujita <etsuro.fuj...@gmail.com> wrote: > > On Wed, Aug 7, 2019 at 10:24 AM Amit Langote <amitlangot...@gmail.com> > > wrote: > > > * Regarding setting ForeignScan.resultRelIndex even for non-direct > > > modifications, maybe that's not a good idea anymore. A foreign table > > > result relation might be involved in a local join, which prevents it > > > from being directly-modifiable and also hides the ForeignScan node > > > from being easily modifiable in PlanForeignModify. Maybe, we should > > > just interpret resultRelIndex as being set only when > > > direct-modification is feasible. > > > > Yeah, I think so; when using PlanForeignModify because for example, > > the foreign table result relation is involved in a local join, as you > > mentioned, ForeignScan.operation would be left unchanged (ie, > > CMD_SELECT), so to me it's more understandable to not set > > ForeignScan.resultRelIndex. > > OK. > > > > Should we rename the field > > > accordingly to be self-documenting? > > > > IMO I like the name resultRelIndex, but do you have any better idea? > > On second thought, I'm fine with sticking to resultRelIndex. Trying > to make it self documenting might make the name very long.
OK > Here are the updated patches. IIUC, I think we reached a consensus at least on the 0001 patch. Andres, would you mind if I commit that patch? Best regards, Etsuro Fujita
