Hi,
I noticed that tuple-routing for partitioned tables that contain
non-writable foreign partitions doesn't work as expected. Here is an
example:
postgres=# create extension file_fdw;
postgres=# create server file_server foreign data wrapper file_fdw;
postgres=# create user mapping for CURRENT_USER server file_server;
postgres=# create table p (a int) partition by list (a);
postgres=# create foreign table t1 partition of p for values in (1)
server file_server options (format 'csv', filename '/path/to/file',
delimiter ',');
postgres=# create table t2 partition of p for values in (2);
postgres=# insert into p values (1);
ERROR: cannot insert into foreign table "t1"
Looks good, but:
postgres=# insert into p values (2);
ERROR: cannot insert into foreign table "t1"
The insert should work but doesn't. (It also seems odd to me that the
error message points to t1, not t2.) The reason for that is
CheckValidResultRel in ExecSetupPartitionTupleRouting aborts any insert
into a partitioned table in the case where the partitioned table
contains at least one foreign partition into which the FDW can't insert.
I don't think that that is intentional behavior, so I'd like to
propose to fix that by skipping CheckValidResultRel for foreign
partitions because we can abort an insert into a foreign partition after
ExecFindPartition in ExecInsert, by checking to see if
resultRelInfo->ri_FdwRoutine is not NULL. Attached is a proposed patch
for that. Since COPY FROM has the same issue, I added regression tests
for COPY FROM as well as INSERT to file_fdw.
Best regards,
Etsuro Fujita
*** /dev/null
--- b/contrib/file_fdw/data/list1.csv
***************
*** 0 ****
--- 1,2 ----
+ 1,foo
+ 1,bar
*** /dev/null
--- b/contrib/file_fdw/data/list2.bad
***************
*** 0 ****
--- 1,2 ----
+ 2,baz
+ 1,qux
*** /dev/null
--- b/contrib/file_fdw/data/list2.csv
***************
*** 0 ****
--- 1,2 ----
+ 2,baz
+ 2,qux
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
***************
*** 162,167 **** SELECT tableoid::regclass, * FROM agg FOR UPDATE;
--- 162,188 ----
ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
DROP TABLE agg;
+ -- declarative partitioning tests
+ SET ROLE regress_file_fdw_superuser;
+ CREATE TABLE pt (a int, b text) partition by list (a);
+ CREATE FOREIGN TABLE p1 partition of pt for values in (1) SERVER file_server
+ OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',');
+ CREATE TABLE p2 partition of pt for values in (2);
+ SELECT tableoid::regclass, * FROM pt;
+ SELECT tableoid::regclass, * FROM p1;
+ SELECT tableoid::regclass, * FROM p2;
+ COPY pt FROM '@abs_srcdir@/data/list2.bad' with (format 'csv', delimiter
','); -- ERROR
+ COPY pt FROM '@abs_srcdir@/data/list2.csv' with (format 'csv', delimiter ',');
+ SELECT tableoid::regclass, * FROM pt;
+ SELECT tableoid::regclass, * FROM p1;
+ SELECT tableoid::regclass, * FROM p2;
+ INSERT INTO pt VALUES (1, 'xyzzy'); -- ERROR
+ INSERT INTO pt VALUES (2, 'xyzzy');
+ SELECT tableoid::regclass, * FROM pt;
+ SELECT tableoid::regclass, * FROM p1;
+ SELECT tableoid::regclass, * FROM p2;
+ DROP TABLE pt;
+
-- privilege tests
SET ROLE regress_file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
***************
*** 289,294 **** SELECT tableoid::regclass, * FROM agg FOR UPDATE;
--- 289,375 ----
ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
DROP TABLE agg;
+ -- declarative partitioning tests
+ SET ROLE regress_file_fdw_superuser;
+ CREATE TABLE pt (a int, b text) partition by list (a);
+ CREATE FOREIGN TABLE p1 partition of pt for values in (1) SERVER file_server
+ OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',');
+ CREATE TABLE p2 partition of pt for values in (2);
+ SELECT tableoid::regclass, * FROM pt;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p1;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p2;
+ tableoid | a | b
+ ----------+---+---
+ (0 rows)
+
+ COPY pt FROM '@abs_srcdir@/data/list2.bad' with (format 'csv', delimiter
','); -- ERROR
+ ERROR: cannot route inserted tuples to a foreign table
+ CONTEXT: COPY pt, line 2: "1,qux"
+ COPY pt FROM '@abs_srcdir@/data/list2.csv' with (format 'csv', delimiter ',');
+ SELECT tableoid::regclass, * FROM pt;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ p2 | 2 | baz
+ p2 | 2 | qux
+ (4 rows)
+
+ SELECT tableoid::regclass, * FROM p1;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p2;
+ tableoid | a | b
+ ----------+---+-----
+ p2 | 2 | baz
+ p2 | 2 | qux
+ (2 rows)
+
+ INSERT INTO pt VALUES (1, 'xyzzy'); -- ERROR
+ ERROR: cannot route inserted tuples to a foreign table
+ INSERT INTO pt VALUES (2, 'xyzzy');
+ SELECT tableoid::regclass, * FROM pt;
+ tableoid | a | b
+ ----------+---+-------
+ p1 | 1 | foo
+ p1 | 1 | bar
+ p2 | 2 | baz
+ p2 | 2 | qux
+ p2 | 2 | xyzzy
+ (5 rows)
+
+ SELECT tableoid::regclass, * FROM p1;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p2;
+ tableoid | a | b
+ ----------+---+-------
+ p2 | 2 | baz
+ p2 | 2 | qux
+ p2 | 2 | xyzzy
+ (3 rows)
+
+ DROP TABLE pt;
-- privilege tests
SET ROLE regress_file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 3281,3290 **** ExecSetupPartitionTupleRouting(Relation rel,
partrel = heap_open(lfirst_oid(cell), NoLock);
part_tupdesc = RelationGetDescr(partrel);
/*
* Verify result relation is a valid target for the current
operation.
*/
! CheckValidResultRel(partrel, CMD_INSERT);
/*
* Save a tuple conversion map to convert a tuple routed to this
--- 3281,3294 ----
partrel = heap_open(lfirst_oid(cell), NoLock);
part_tupdesc = RelationGetDescr(partrel);
+ Assert(partrel->rd_rel->relkind == RELKIND_RELATION ||
+ partrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE);
+
/*
* Verify result relation is a valid target for the current
operation.
*/
! if (partrel->rd_rel->relkind == RELKIND_RELATION)
! CheckValidResultRel(partrel, CMD_INSERT);
/*
* Save a tuple conversion map to convert a tuple routed to this
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
