(2019/04/23 4:37), Laurenz Albe wrote:
On Mon, 2019-04-22 at 21:45 +0900, Etsuro Fujita wrote:
(2019/04/20 20:53), Laurenz Albe wrote:
On Fri, 2018-04-06 at 23:24 +0000, Robert Haas wrote:
Allow insert and update tuple routing and COPY for foreign tables.
Also enable this for postgres_fdw.
Etsuro Fujita, based on an earlier patch by Amit Langote. The larger
patch series of which this is a part has been reviewed by Amit
Langote, David Fetter, Maksim Milyutin, Álvaro Herrera, Stephen Frost,
and me. Minor documentation changes to the final version by me.
Discussion:
http://postgr.es/m/29906a26-da12-8c86-4fb9-d8f88442f...@lab.ntt.co.jp
If a FDW implements ExecForeignInsert, this commit automatically assumes
that it also supports COPY FROM. It will call ExecForeignInsert without
calling PlanForeignModify and BeginForeignModify, and a FDW that does not
expect that will probably fail.
This is not 100% correct; the FDW documentation says:
<para>
Tuples inserted into a partitioned table by
<command>INSERT</command> or
<command>COPY FROM</command> are routed to partitions. If an FDW
supports routable foreign-table partitions, it should also provide the
following callback functions. These functions are also called when
<command>COPY FROM</command> is executed on a foreign table.
</para>
I don't see the difference between the documentation and what I wrote above.
Before v11, a FDW could expect that ExecForeignInsert is only called if
BeginForeignModify was called earlier.
That has silently changed with v11.
I have to admit that the documentation is not sufficient.
It's permissible to throw an error in BeginForeignInsert, so what I was
thinking for FDWs that don't want to support COPY FROM and
INSERT/UPDATE/COPY FROM tuple routing was to provide BeginForeignInsert
implementing something like this:
static void
fooBeginForeignInsert(ModifyTableState *mtstate,
ResultRelInfo *resultRelInfo)
{
Relation rel = resultRelInfo->ri_RelationDesc;
if (mtstate->ps.plan == NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot copy to foreign table \"%s\"",
RelationGetRelationName(rel))));
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot route tuples into foreign table \"%s\"",
RelationGetRelationName(rel))));
}
Sure, it is not hard to modify a FDW to continue working with v11.
How about adding to the documentation for BeginForeignInsert a mention
that if an FDW doesn't support COPY FROM and/or routable foreign tables,
it must throw an error in BeginForeignInsert accordingly.
My point is that this should not be necessary.
In my opinion, I think this is necessary...
On the other hand, if a FDW wants to support COPY in v11 and has no
need for BeginForeignInsert to support that, it is a simple exercise
for it to provide an empty BeginForeignInsert just to signal that it
wants to support COPY.
That seems to me inconsistent with the concept of the existing APIs for
updating foreign tables, because for an FDW that wants to support
INSERT/UPDATE/DELETE and has no need for
PlanForeignModify/BeginForeignModify, those APIs don't require the FDW
to provide empty PlanForeignModify/BeginForeignModify to tell the core
that it wants to support INSERT/UPDATE/DELETE.
Best regards,
Etsuro Fujita
