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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Maksim Milyutin <milyutinma(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Add support for tuple routing to foreign partitions
Date: 2018-04-05 10:21:05
Message-ID: 5AC5F891.8020600@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(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

Attachment Content-Type Size
0001_postgres-fdw-refactoring-6.patch text/x-diff 11.3 KB
0003_foreign-routing-fdwapi-6.patch text/x-diff 54.0 KB
0002_copy-from-check-constraint-fix.patch text/x-diff 667 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Teodor Sigaev 2018-04-05 10:23:17 Re: [HACKERS] GUC for cleanup indexes threshold.
Previous Message Simon Riggs 2018-04-05 10:15:20 Re: pgsql: New files for MERGE