pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Robert Haas <rhaas@postgresql.org>
To: pgsql-committers@lists.postgresql.org
To: pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Allow insert and update tuple routing and COPY for foreign table
Date: Sat, 20 Apr 2019 13:53:26 +0200
Message-ID: <ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at> (raw)
In-Reply-To: <E1f4aif-0004qo-A6@gemulon.postgresql.org>
References: <E1f4aif-0004qo-A6@gemulon.postgresql.org>

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

This commit makes life hard for foreign data wrappers that support
data modifications.

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.

So this commit silently turns a functioning FDW into a broken FDW.
That is not nice.  Probably not every FDW is aware of this change, and
maybe there are FDWs that support INSERT but don't want to support COPY
for some reason.

I propose that PostgreSQL only allows COPY FROM on a foreign table if the FDW
implements BeginForeignInsert.  The attached patch implements that.

I think this should be backpatched to v11.

Yours,
Laurenz Albe

Attachments:

0001-COPY-FROM-on-foreign-tables-requires-BeginForeignIns.patch (text/x-patch, 2.1K) download From c4b0e871658c757124dad992578da0b60fccf962 Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.albe@cybertec.at> Date: Sat, 20 Apr 2019 13:36:56 +0200 Subject: [PATCH] COPY FROM on foreign tables requires BeginForeignInsert Commit 3d956d956a introduced COPY FROM support for foreign tables. If a foreign data wrapper supports data modifications, but either has not adapted to this change yet or doesn't want to support COPY FROM for other reasons, it probably got broken by the above commit, because COPY will just call ExecForeignInsert anyway, which might not work because neither PlanForeignModify nor BeginForeignModify have been called. To avoid breaking third-party foreign data wrappers in that way, allow COPY FROM for foreign tables only if the foreign data wrapper implements BeginForeignInsert. --- doc/src/sgml/fdwhandler.sgml | 2 ++ src/backend/commands/copy.c | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 2c07a86637..8cd174a204 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -724,6 +724,8 @@ BeginForeignInsert(ModifyTableState *mtstate, perform any initialization needed prior to the actual insertion. Subsequently, <function>ExecForeignInsert</function> will be called for each tuple to be inserted into the foreign table. + All foreign data wrappers that support <command>COPY FROM</command> have + to provide this callback function. </para> <para> diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index c39218f8db..944d558cd4 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2857,6 +2857,11 @@ CopyFrom(CopyState cstate) resultRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL) resultRelInfo->ri_FdwRoutine->BeginForeignInsert(mtstate, resultRelInfo); + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE NOT SUPPORTED), + errmsg("cannot copy to foreign table \"%s\"", + RelationGetRelationName(cstate->rel)))); /* Prepare to catch AFTER triggers. */ AfterTriggerBeginQuery(); -- 2.20.1

view thread (22+ messages)  latest in thread

Message-ID: <ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at>
Permalink:  ../ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at/
Also on:    postgresql.org/message-id/ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at

 ·  · 

reply

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: 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: <ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at>

* 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