Re: partition routing layering in nodeModifyTable.c

On Wed, Oct 14, 2020 at 1:30 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 13/10/2020 19:09, Heikki Linnakangas wrote:
> > One little idea I had:
> >
> > I think all FDWs that support direct modify will have to carry the
> > resultRelaton index or the ResultRelInfo pointer from BeginDirectModify
> > to IterateDirectModify in the FDW's private struct. It's not
> > complicated, but should we make life easier for FDWs by storing the
> > ResultRelInfo pointer in the ForeignScanState struct in the core code?
> > The doc now says:
> >
> >> The data that was actually inserted, updated or deleted must be
> >> stored in the ri_projectReturning->pi_exprContext->ecxt_scantuple of
> >> the target foreign table's ResultRelInfo obtained using the
> >> information passed to BeginDirectModify. Return NULL if no more rows
> >> are available.
> >
> > That "ResultRelInfo obtained using the information passed to
> > BeginDirectModify" part is a pretty vague. We could expand it, but if we
> > stored the ResultRelInfo in the ForeignScanState, we could explain it
> > succinctly.
>
> I tried that approach, see attached. Yeah, this feels better to me.

I like the idea of storing the ResultRelInfo in ForeignScanState, but
it would be better if we can document the fact that an FDW may not
reliably access until IterateDirectModify(). That's because, setting
it in ExecInitForeignScan() will mean *all* result relations must be
initialized during ExecInitModifyTable(), which defies my
lazy-ResultRelInfo-initiailization proposal.  As to why why I'm
pushing that proposal, consider that when we'll get the ability to use
run-time pruning for UPDATE/DELETE with [1], initializing all result
relations before initializing the plan tree will mean most of those
ResultRelInfos will be unused, because run-time pruning that occurs
when the plan tree is initialized (and/or when it is executed) may
eliminate most but a few result relations.

I've attached a diff to v17-0001 to show one way of delaying setting
ForeignScanState.resultRelInfo.

-- 
Amit Langote
EDB: http://www.enterprisedb.com

[1] https://commitfest.postgresql.org/30/2575/