Hi,
On Wed, Aug 7, 2019 at 11:47 AM Amit Langote <amitlangote09@gmail.com> wrote:
> On Wed, Aug 7, 2019 at 11:30 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Wed, Aug 7, 2019 at 10:24 AM Amit Langote <amitlangote09@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