Fujita-san, On 2017/08/07 12:45, Etsuro Fujita wrote: > 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.
Thanks for reporting this. All the issues you mention here seem valid to me. So, once we add support in general for foreign partition tuple-routing, we'd still get the same error ("cannot route to foreign partitions") in the file_fdw's case, but only because resultRelInfo->ri_FdwRoutine->ExecForeignInsert will be NULL for file_fdw. The patch looks good too. Although, looking at the following hunk: + 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); makes me now wonder if we need the CheckValidResultRel check at all. The only check currently in place for RELKIND_RELATION is CheckCmdReplicaIdentity(), which is a noop (returns true) for inserts anyway. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers