From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Etsuro Fujita <fujita(dot)etsuro(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 07:31:35 |
Message-ID: | 732fae79-39b8-c0f7-678c-8e6911d722fc@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2018-04-05 07:55:50 | Re: [HACKERS] MERGE SQL Statement for PG11 |
Previous Message | Heikki Linnakangas | 2018-04-05 07:23:43 | Excessive PostmasterIsAlive calls slow down WAL redo |