From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Laurenz Albe <laurenz.albe@cybertec.at>
Cc: Robert Haas <rhaas@postgresql.org>
Cc: pgsql-committers@lists.postgresql.org
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Allow insert and update tuple routing and COPY for foreign table
Date: Wed, 24 Apr 2019 20:54:26 +0900
Message-ID: <5CC04E72.90405@lab.ntt.co.jp> (raw)
In-Reply-To: <b459d1b1b4eb8c0d538a2c5b70e131af3c391954.camel@cybertec.at>
References: <E1f4aif-0004qo-A6@gemulon.postgresql.org>
<ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at>
<5CBDB770.2060602@lab.ntt.co.jp>
<b459d1b1b4eb8c0d538a2c5b70e131af3c391954.camel@cybertec.at>
(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-d8f88442f2b9@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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: fujita.etsuro@lab.ntt.co.jp, laurenz.albe@cybertec.at, rhaas@postgresql.org, pgsql-committers@lists.postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Allow insert and update tuple routing and COPY for foreign table
In-Reply-To: <5CC04E72.90405@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox