postgres_fdw: Add support for INSERT OVERRIDING clause
It has been pointed out to me that the command deparsing in postgres_fdw
does not support the INSERT OVERRIDING clause that was added in PG10.
Here is a patch that seems to fix that. I don't know much about this,
whether anything else needs to be added or whether there should be
tests. Perhaps someone more familiar with postgres_fdw details can
check it.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-postgres_fdw-Add-support-for-INSERT-OVERRIDING-claus.patchtext/plain; charset=UTF-8; name=0001-postgres_fdw-Add-support-for-INSERT-OVERRIDING-claus.patch; x-mac-creator=0; x-mac-type=0Download
From 097ecf61a9c2740b02a21be19a92aed92388346a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 31 Oct 2017 11:44:14 -0400
Subject: [PATCH] postgres_fdw: Add support for INSERT OVERRIDING clause
---
contrib/postgres_fdw/deparse.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 2af8364010..8eb227605f 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1573,7 +1573,21 @@ deparseInsertSql(StringInfo buf, PlannerInfo *root,
deparseColumnRef(buf, rtindex, attnum, root, false);
}
- appendStringInfoString(buf, ") VALUES (");
+ appendStringInfoString(buf, ") ");
+
+ switch (root->parse->override)
+ {
+ case OVERRIDING_USER_VALUE:
+ appendStringInfoString(buf, "OVERRIDING USER VALUE ");
+ break;
+ case OVERRIDING_SYSTEM_VALUE:
+ appendStringInfoString(buf, "OVERRIDING SYSTEM VALUE ");
+ break;
+ default:
+ break;
+ }
+
+ appendStringInfoString(buf, "VALUES (");
pindex = 1;
first = true;
--
2.14.3
On Tue, Oct 31, 2017 at 3:45 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
It has been pointed out to me that the command deparsing in postgres_fdw
does not support the INSERT OVERRIDING clause that was added in PG10.
Here is a patch that seems to fix that. I don't know much about this,
whether anything else needs to be added or whether there should be
tests. Perhaps someone more familiar with postgres_fdw details can
check it.
Trying to insert some data using OVERRIDING SYSTEM VALUE on a foreign
table with a remote relation defined with GENERATED ALWAYS would just
fail:
=# insert into id_always_foreign OVERRIDING system VALUE values (8);
ERROR: 428C9: cannot insert into column "a"
DETAIL: Column "a" is an identity column defined as GENERATED ALWAYS.
HINT: Use OVERRIDING SYSTEM VALUE to override.
And that's confusing because there is no actual way to avoid this
error if postgres_fdw is unpatched.
I think that you should add some tests, and make sure that the
documentation of postgres-fdw.sgml mentions that those two clauses are
pushed down.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 11/3/17 07:53, Michael Paquier wrote:
Trying to insert some data using OVERRIDING SYSTEM VALUE on a foreign
table with a remote relation defined with GENERATED ALWAYS would just
fail:
=# insert into id_always_foreign OVERRIDING system VALUE values (8);
ERROR: 428C9: cannot insert into column "a"
DETAIL: Column "a" is an identity column defined as GENERATED ALWAYS.
HINT: Use OVERRIDING SYSTEM VALUE to override.And that's confusing because there is no actual way to avoid this
error if postgres_fdw is unpatched.I think that you should add some tests, and make sure that the
documentation of postgres-fdw.sgml mentions that those two clauses are
pushed down.
I've been playing with a few test cases and I'm a bit confused by how
some of this is supposed to work. AFAICT, in the SQL standard, foreign
tables can't have column defaults, but in PostgreSQL it's allowed. This
creates some semantic differences, I think. For example, if I do this
in the postgres_fdw.sql test file:
create table loc2 (f1 int generated always as identity, f2 text);
create foreign table rem2 (f1 int, f2 text)
server loopback options(table_name 'loc2');
insert into rem2(f2) values('hi remote');
then we get the error
ERROR: cannot insert into column "f1"
DETAIL: Column "f1" is an identity column defined as GENERATED ALWAYS.
probably because it resolves f1 on the local server and then sends an
explicit NULL to insert on the remote.
I think, however, that it would be more appropriate to send DEFAULT and
let the remote side sort it out. That way, this command would work
transparently independent of how the default is defined on the remote
side. AFAICT, it is not possible to do that.
Is this defined or explained anywhere?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
I've been playing with a few test cases and I'm a bit confused by how
some of this is supposed to work. AFAICT, in the SQL standard, foreign
tables can't have column defaults, but in PostgreSQL it's allowed. This
creates some semantic differences, I think. For example, if I do this
in the postgres_fdw.sql test file:
create table loc2 (f1 int generated always as identity, f2 text);
create foreign table rem2 (f1 int, f2 text)
server loopback options(table_name 'loc2');
insert into rem2(f2) values('hi remote');
Note that this example has nothing to do with any non-standard
extensions: rem2 hasn't got a default.
then we get the error
ERROR: cannot insert into column "f1"
DETAIL: Column "f1" is an identity column defined as GENERATED ALWAYS.
probably because it resolves f1 on the local server and then sends an
explicit NULL to insert on the remote.
I think, however, that it would be more appropriate to send DEFAULT and
let the remote side sort it out. That way, this command would work
transparently independent of how the default is defined on the remote
side. AFAICT, it is not possible to do that.
Is this defined or explained anywhere?
IIRC, this issue was debated at great length back when we first put
in foreign tables, because early drafts of postgres_fdw did what you
propose here, and we ran into very nasty problems. We eventually decided
that allowing remotely-determined column defaults was a can of worms we
didn't want to open. I do not think that GENERATED columns really change
anything about that. They certainly don't do anything to resolve the
problems we were contending with back then. (Which I don't recall the
details of; you'll need to trawl the archives. Should be somewhere early
in 2013, though, since we implemented that change in commit 50c19fc76.)
regards, tom lane
On Wed, Nov 29, 2017 at 8:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
IIRC, this issue was debated at great length back when we first put
in foreign tables, because early drafts of postgres_fdw did what you
propose here, and we ran into very nasty problems. We eventually decided
that allowing remotely-determined column defaults was a can of worms we
didn't want to open. I do not think that GENERATED columns really change
anything about that. They certainly don't do anything to resolve the
problems we were contending with back then. (Which I don't recall the
details of; you'll need to trawl the archives. Should be somewhere early
in 2013, though, since we implemented that change in commit 50c19fc76.)
So this gives a good reason to do nothing or return an error at
postgres_fdw level for OVERRIDING?
--
Michael
On Wed, Nov 29, 2017 at 1:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Wed, Nov 29, 2017 at 8:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
IIRC, this issue was debated at great length back when we first put
in foreign tables, because early drafts of postgres_fdw did what you
propose here, and we ran into very nasty problems. We eventually decided
that allowing remotely-determined column defaults was a can of worms we
didn't want to open. I do not think that GENERATED columns really change
anything about that. They certainly don't do anything to resolve the
problems we were contending with back then. (Which I don't recall the
details of; you'll need to trawl the archives. Should be somewhere early
in 2013, though, since we implemented that change in commit 50c19fc76.)So this gives a good reason to do nothing or return an error at
postgres_fdw level for OVERRIDING?
Moving the patch to next CF as the discussion has not settled yet.
--
Michael
On 11/29/17 19:59, Michael Paquier wrote:
On Wed, Nov 29, 2017 at 1:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Wed, Nov 29, 2017 at 8:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
IIRC, this issue was debated at great length back when we first put
in foreign tables, because early drafts of postgres_fdw did what you
propose here, and we ran into very nasty problems. We eventually decided
that allowing remotely-determined column defaults was a can of worms we
didn't want to open. I do not think that GENERATED columns really change
anything about that. They certainly don't do anything to resolve the
problems we were contending with back then. (Which I don't recall the
details of; you'll need to trawl the archives. Should be somewhere early
in 2013, though, since we implemented that change in commit 50c19fc76.)So this gives a good reason to do nothing or return an error at
postgres_fdw level for OVERRIDING?Moving the patch to next CF as the discussion has not settled yet.
I think I'll close this patch. I was operating under the assumption
that there is a bug of omission in PG10 here. But it seems this
combination of features just isn't meant to work together at this time.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services