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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
Cc: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Robert Haas <rhaas(at)postgresql(dot)org>, pgsql-committers(at)lists(dot)postgresql(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Allow insert and update tuple routing and COPY for foreign table
Date: 2019-04-22 21:07:51
Message-ID: 20190422210751.sbgtmweuaodcu2er@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Hi,

On 2019-04-22 22:56:20 +0200, Laurenz Albe wrote:
> On Mon, 2019-04-22 at 13:27 -0700, Andres Freund wrote:
> > On 2019-04-22 21:37:25 +0200, Laurenz Albe wrote:
> > > Commit 3d956d956a introduced support for foreign tables as partitions
> > > and COPY FROM on 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 it
> > > 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 and tuple routing for foreign tables only if the foreign data
> > > wrapper implements BeginForeignInsert.
> >
> > Isn't this worse though? Before this it's an API change between major
> > versions. With this it's an API change in a *minor* version. Sure, it's
> > one that doesn't crash, but it's still a pretty substantial function
> > regression, no?
>
> Hm, that's a good point.
> You could say that this patch is too late, because a FDW might already be
> relying on COPY FROM to work without BeginForeignInsert in v11.

I think that's the case.

> How about just applying the patch from v12 on?
> Then it is a behavior change in a major release, which is acceptable.
> It requires the imaginary FDW above to add an empty BeginForeignInsert
> callback function, but will unbreak FDW that slept through the change
> that added COPY support.

I fail to see the advantage. It'll still require FDWs to be fixed to
work correctly for v11, but additionally adds another set of API
differences that needs to be fixed by another set of FDWs. I think this
ship simply has sailed.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2019-04-22 21:51:40 pgsql: Convert gist to compute page level xid horizon on primary.
Previous Message Laurenz Albe 2019-04-22 20:56:20 Re: pgsql: Allow insert and update tuple routing and COPY for foreign table

Browse pgsql-hackers by date

  From Date Subject
Next Message Juan José Santamaría Flecha 2019-04-22 21:41:57 Re: TM format can mix encodings in to_char()
Previous Message Laurenz Albe 2019-04-22 20:56:20 Re: pgsql: Allow insert and update tuple routing and COPY for foreign table