Re: pgsql: Allow insert and update tuple routing and COPY for foreign table

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