Re: partition routing layering in nodeModifyTable.c

Hi,

On 2019-08-05 18:16:10 +0900, Amit Langote wrote:
> The patch adds a resultRelIndex field to ForeignScan node, which is
> set to >= 0 value for non-SELECT queries. I first thought to set it
> only if direct modification is being used, but maybe it'd be simpler
> to set it even if direct modification is not used.

Yea, I think we should just always set it.

> To set it, the
> patch teaches set_plan_refs() to initialize resultRelIndex of
> ForeignScan plans that appear under ModifyTable. Fujita-san said he
> plans to revise the planning of direct-modification style queries to
> not require a ModifyTable node anymore, but maybe he'll just need to
> add similar code elsewhere but not outside setrefs.c.

I think I prefer the approach in Fujita-san's email. While not extremely
pretty either, it would allow for having nodes between the foreign scan
and the modify node.

> > Then we could just have BeginForeignModify, BeginDirectModify,
> > BeginForeignScan all be called from ExecInitForeignScan().
>
> I too think that it would've been great if we could call both
> BeginForeignModify and BeginDirectModify from ExecInitForeignScan, but
> the former's API seems to be designed to be called from
> ExecInitModifyTable from the get-go. Maybe we should leave that
> as-is?

Yea, we should leave it where it is. I think the API here is fairly
ugly, but it's probably not worth changing. And if we were to change it,
it'd need a lot bigger hammer.

Greetings,

Andres Freund