9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

Started by Peter Geogheganover 10 years ago8 messages
#1Peter Geoghegan
pg@heroku.com

postgres_fdw supports ON CONFLICT DO NOTHING, provided no inference
specification is provided. Foreign tables do not have associated
unique indexes (or exclusion constraints) as far as the optimizer is
concerned, and so Postgres does not accept an inference specification
for foreign tables -- the optimizer will simply complain that a unique
index that satisfies the user's inference specification is
unavailable.

There is no support for ON CONFLICT DO UPDATE with postgres_fdw, but
that's really only because an inference specification (or explicitly
named constraint) is always required for DO UPDATE. The deparsing
support actually added will have deparsing add "ON CONFLICT DO
NOTHING" for the SQL generated for execution on foreign servers if the
original statement had that exact, unadorned ON CONFLICT clause. As
things stand, every other possible ON CONFLICT clause will throw an
error in some way before the FDW is consulted at all, so FDW authors
need not concern themselves with those other cases (unless perhaps we
allow ON CONFLICT DO UPDATE to not require an inference specification
in a last minute behavioral tweak, as suggested by Simon Riggs, making
ON CONFLICT DO UPDATE support by foreign data wrappers a possibility
that must be considered).

postgres_fdw handles the one simple ON CONFLICT DO NOTHING case (the
only case that can actually reach it), but no other FDW we ship pay
any attention. Do we need to make existing contrib FDWs, like
file_fdw, explicitly reject unadorned ON CONFLICT DO NOTHING clauses
as wrong-headed? Is it okay to just let them not pay attention at all
on the theory that it's the same as performing INSERT ... ON CONFLICT
DO NOTHING on a table that has no unique indexes or exclusion
constraints?

In any case, third party foreign data wrappers that target other
database system will totally ignore ON CONFLICT DO NOTHING when built
against the master branch (unless they consider these questions). They
should perhaps make a point of rejecting DO NOTHING outright where it
makes sense that support could exist, but it just doesn't. Or they
could just add support (I imagine that this would be very easy for
mysql_fdw, for example -- MySQL has INSERT IGNORE). I feel a
compatibility item in the release notes is in order so the question is
considered, but there seems to be no place to do that on the Wiki, and
the original commit message does not have a note like this.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Peter Geoghegan
pg@heroku.com
In reply to: Peter Geoghegan (#1)
Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

On Sun, May 24, 2015 at 4:22 PM, Peter Geoghegan <pg@heroku.com> wrote:

As things stand, every other possible ON CONFLICT clause will throw an
error in some way before the FDW is consulted at all, so FDW authors
need not concern themselves with those other cases (unless perhaps we
allow ON CONFLICT DO UPDATE to not require an inference specification
in a last minute behavioral tweak, as suggested by Simon Riggs, making
ON CONFLICT DO UPDATE support by foreign data wrappers a possibility
that must be considered).

AddForeignUpdateTargets() actually won't be called with ON CONFLICT DO
UPDATE, and so it isn't exactly true that the only obstacle to making
FDWs support ON CONFLICT DO UPDATE is around inference of arbiter
unique indexes on the foreign side. It's *almost* true, though.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Peter Geoghegan
pg@heroku.com
In reply to: Peter Geoghegan (#2)
1 attachment(s)
Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

On Sun, May 24, 2015 at 5:16 PM, Peter Geoghegan <pg@heroku.com> wrote:

AddForeignUpdateTargets() actually won't be called with ON CONFLICT DO
UPDATE, and so it isn't exactly true that the only obstacle to making
FDWs support ON CONFLICT DO UPDATE is around inference of arbiter
unique indexes on the foreign side. It's *almost* true, though.

Attached patch clears this up within the fdw-handler documentation. I
think it's worth separately noting from the existing commentary on
limitations with FDWs and ON CONFLICT.

--
Peter Geoghegan

Attachments:

fdw-handler-arbiter-note.patchtext/x-patch; charset=US-ASCII; name=fdw-handler-arbiter-note.patchDownload
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 2361577..569a22d 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -399,6 +399,13 @@ AddForeignUpdateTargets (Query *parsetree,
     </para>
 
     <para>
+     Note that <function>AddForeignUpdateTargets</> will not be called
+     for <command>INSERT</> operations with an <literal>ON CONFLICT DO
+     UPDATE</> clause.  Such <command>INSERT</> operations are
+     unsupported when a foreign table is targeted.
+    </para>
+
+    <para>
 <programlisting>
 List *
 PlanForeignModify (PlannerInfo *root,
#4Simon Riggs
simon@2ndQuadrant.com
In reply to: Peter Geoghegan (#1)
Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

On 25 May 2015 at 00:22, Peter Geoghegan <pg@heroku.com> wrote:

There is no support for ON CONFLICT DO UPDATE with postgres_fdw, but
that's really only because an inference specification (or explicitly
named constraint) is always required for DO UPDATE. The deparsing
support actually added will have deparsing add "ON CONFLICT DO
NOTHING" for the SQL generated for execution on foreign servers if the
original statement had that exact, unadorned ON CONFLICT clause. As
things stand, every other possible ON CONFLICT clause will throw an
error in some way before the FDW is consulted at all, so FDW authors
need not concern themselves with those other cases (unless perhaps we
allow ON CONFLICT DO UPDATE to not require an inference specification
in a last minute behavioral tweak, as suggested by Simon Riggs, making
ON CONFLICT DO UPDATE support by foreign data wrappers a possibility
that must be considered).

My earlier summary was that the support for multiple constraints has been
poorly thought through. This is an example of the breakage I have been
complaining about when we are forced to specify the constraint
(conflict-target).

This is not just related to FDWs and should not be fixed solely for FDWs.
This was already an open item for me in 9.5, now even more so.

My comments do not come at the last minute, what Peter means is that we
should make a change now in response to the concerns I have previously
raised.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Albe Laurenz
laurenz.albe@wien.gv.at
In reply to: Peter Geoghegan (#1)
Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

Peter Geoghegan wrote:

In any case, third party foreign data wrappers that target other
database system will totally ignore ON CONFLICT DO NOTHING when built
against the master branch (unless they consider these questions). They
should perhaps make a point of rejecting DO NOTHING outright where it
makes sense that support could exist, but it just doesn't. Or they
could just add support (I imagine that this would be very easy for
mysql_fdw, for example -- MySQL has INSERT IGNORE). I feel a
compatibility item in the release notes is in order so the question is
considered, but there seems to be no place to do that on the Wiki, and
the original commit message does not have a note like this.

+1

I wouldn't have become aware of that if I hadn't read your message.

Yours,
Laurenz Albe

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Peter Geoghegan (#2)
Re: Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

On 2015/05/25 9:16, Peter Geoghegan wrote:

AddForeignUpdateTargets() actually won't be called with ON CONFLICT DO
UPDATE, and so it isn't exactly true that the only obstacle to making
FDWs support ON CONFLICT DO UPDATE is around inference of arbiter
unique indexes on the foreign side. It's *almost* true, though.

I think that those are interesting problems. Wouldn't we need some
additional hacks for the core or FDW to perform an operation that is
equivalent to dynamically switching the ExecInsert/ExecForeignInsert
processing to the ExecUpdate/ExecForeignUpdate processing in case of a
conflict?

Best regards,
Etsuro Fujita

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Peter Geoghegan
pg@heroku.com
In reply to: Etsuro Fujita (#6)
Re: Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

On Thu, May 28, 2015 at 1:20 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:

I think that those are interesting problems. Wouldn't we need some
additional hacks for the core or FDW to perform an operation that is
equivalent to dynamically switching the ExecInsert/ExecForeignInsert
processing to the ExecUpdate/ExecForeignUpdate processing in case of a
conflict?

I did not imagine so. Rather, I thought that it was a matter of simply
introducing a way that foreign tables can have foreign constraints
recognizable by the local Postgres optimizer. The decision to insert
or update must belong to the foreign server, since the feature could
be useful for systems like MySQL, and not just Postgres. I may be
mistaken.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Peter Geoghegan
pg@heroku.com
In reply to: Simon Riggs (#4)
Re: 9.5 release notes may need ON CONFLICT DO NOTHING compatibility notice for FDW authors

On Mon, May 25, 2015 at 1:28 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

My earlier summary was that the support for multiple constraints has been
poorly thought through. This is an example of the breakage I have been
complaining about when we are forced to specify the constraint
(conflict-target).

This is not just related to FDWs and should not be fixed solely for FDWs.
This was already an open item for me in 9.5, now even more so.

I agree that the decision to change the current behavior has nothing
to do with FDWs. There is no reason to treat foreign tables
differently to local ones in this regard, which implies that ON
CONFLICT DO UPDATE cannot work with postgres_fdw unless and until
someone invents foreign constraints on foreign tables (I think), or
unless we change our mind generally (for other reasons). So,
certainly, the rationale for mandating (or not mandating) an inference
specification with ON CONFLICT DO UPDATE ought to come from balancing
concerns about safety, compatibility, flexibility, and so on.

I did not mean to imply that your comments were unreasonable/too late.
However, I don't see a lot of demand for changing the behavior. There
is at least some demand for accepting as arbiters multiple unique
constraints (that are not more or less equivalent), from Andres for
example, but that's a different question. It's also something that
could reasonably be added later.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers