Re: [HACKERS] Add support for tuple routing to foreign partitions

Fuiita-san,

On 2018/04/05 15:56, Etsuro Fujita wrote:
> (2018/04/05 15:37), Amit Langote wrote:
>> I noticed that the 2nd patch (foreign-routing-fdwapi-5.patch) fails to
>> apply to copy.c:
>
> I forgot to mention this: the second patch is created on top of the first
> patch (postgres-fdw-refactoring-5.patch) and the patch in [1] as before.

Ah, sorry I hadn't noticed that in your previous email.

Might be a good idea to attach the bug-fix patch here as well, and perhaps
add numbers to the file names like:

0001_postgres-fdw-refactoring-5.patch
0002_BUGFIX-copy-from-check-constraint-fix.patch
0003_foreign-routing-fdwapi-5.patch

Just one minor comment:

I wonder why you decided not to have the CheckValidResultRel() call and
the statement that sets ri_PartitionReadyForRouting inside the newly added
ExecInitRoutingInfo itself. If ExecInitRoutingInfo does the last
necessary steps for a ResultRelInfo (and hence the partition) to be ready
to be used for routing, why not finish everything there. So the changes
to ExecPrepareTupleRouting which look like this in the patch:

+ if (!partrel->ri_PartitionReadyForRouting)
+ {
+ CheckValidResultRel(partrel, CMD_INSERT);
+
+ /* Set up information needed for routing tuples to the partition */
+ ExecInitRoutingInfo(mtstate, estate, proute, partrel, partidx);
+
+ partrel->ri_PartitionReadyForRouting = true;
+ }

will become:

+ if (!partrel->ri_PartitionReadyForRouting)
+ ExecInitRoutingInfo(mtstate, estate, proute, partrel, partidx);

As I see no other issues, I will mark this as Ready for Committer.

Thanks,
Amit