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

Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>

From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Stephen Frost <sfrost@snowman.net>, Maksim Milyutin <milyutinma@gmail.com>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2018-04-05T10:21:05Z
Lists: pgsql-hackers

Attachments

(2018/04/05 16:31), Amit Langote wrote:
> 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

OK

> 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);

Good idea!  Modified that way.

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

Thanks!

Attached is an updated version of the patch set plus the patch in [1]. 
Patch 0003_foreign-routing-fdwapi-6.patch can be applied on top of patch 
0001_postgres-fdw-refactoring-6.patch and 
0002_copy-from-check-constraint-fix.patch.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/message-id/5ABA4074.1090500@lab.ntt.co.jp

Commits

  1. Allow insert and update tuple routing and COPY for foreign tables.

  2. Refactor PgFdwModifyState creation/destruction into separate functions.