Subject: Re: pgsql: Allow insert and update tuple routing and COPY for foreign
 table
References: <E1f4aif-0004qo-A6@gemulon.postgresql.org>
 <ed0b7471dce91169d871347cf003d8094ab96895.camel@cybertec.at>
 <5CBDB770.2060602@lab.ntt.co.jp>
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Message-ID: <44ab32f8-bec7-86b6-658d-b4174ebbdd6f@lab.ntt.co.jp>
Date: Tue, 23 Apr 2019 16:44:50 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <5CBDB770.2060602@lab.ntt.co.jp>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>,
        Laurenz Albe <laurenz.albe@cybertec.at>
Cc: Robert Haas <rhaas@postgresql.org>, pgsql-committers@lists.postgresql.org,
        pgsql-hackers@lists.postgresql.org
Precedence: bulk

On 2019/04/22 21:45, Etsuro Fujita wrote:
> (2019/04/20 20:53), Laurenz Albe wrote:
>> I propose that PostgreSQL only allows COPY FROM on a foreign table if
>> the FDW
>> implements BeginForeignInsert.  The attached patch implements that.
> 
> I don't think that is a good idea, because there might be some FDWs that
> want to support COPY FROM on foreign tables without providing
> BeginForeignInsert.  (As for INSERT into foreign tables, we actually allow
> FDWs to support it without providing PlanForeignModify,
> BeginForeignModify, or EndForeignModify.)

I now understand why Laurenz's patch would in fact be a regression for
FDWs that do support COPY FROM and partition tuple routing without
providing BeginForeignInsert, although my first reaction was the opposite,
which was based on thinking (without confirming) that it's the core that
would crash due to initialization step being absent, but that's not the case.

The documentation [1] also says:

  If the BeginForeignInsert pointer is set to NULL, no action is taken for
  the initialization.

[1]
https://www.postgresql.org/docs/current/fdw-callbacks.html#FDW-CALLBACKS-UPDATE

> 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))));
> }

+1

Thanks,
Amit