Horton

Re: partition routing layering in nodeModifyTable.c

Amit Langote <amitlangote09@gmail.com>

View thread View on postgr.es
From: Amit Langote <amitlangote09@gmail.com>
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@gmail.com>, David Rowley <david.rowley@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>, PostgreSQL-development <pgsql-hackers@postgresql.org>, amitdkhan.pg@gmail.com
Date: 2019-08-05T04:31:09Z
Lists: pgsql-hackers 
Hi,

On Sun, Aug 4, 2019 at 4:45 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Sun, Aug 4, 2019 at 3:03 AM Andres Freund <andres@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()?

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.  That information is the ordinal
position of a given result relation in PlannedStmt.resultRelations,
not the RT index as we were discussing.

Thanks,
Amit

Commits

  1. Fix initialization of es_result_relations in EvalPlanQualStart().

  2. Remove PartitionRoutingInfo struct.

  3. Revise child-to-root tuple conversion map management.

  4. Clean up code to resolve the "root target relation" in nodeModifyTable.c

  5. Refactor code for cross-partition updates to a separate function.

  6. Remove es_result_relation_info from EState.

  7. Include result relation info in direct modify ForeignScan nodes.

  8. Create ResultRelInfos later in InitPlan, index them by RT index.