Oddity in tuple routing for foreign partitions

Hi,

While updating the fix-postgres_fdw-WCO-handling patch, I noticed that
the patch for tuple routing for foreign partitions doesn't work well
with remote triggers.  Here is an example:

postgres=# create table loct1 (a int check (a in (1)), b text);
postgres=# create function br_insert_trigfunc() returns trigger as
$$begin new.b := new.b || ' triggered !'; return new; end$$ language
plpgsql;
postgres=# create trigger loct1_br_insert_trigger before insert on loct1
for each row execute procedure br_insert_trigfunc();
postgres=# create table itrtest (a int, b text) partition by list (a);
postgres=# create foreign table remp1 (a int check (a in (1)), b text)
server loopback options (table_name 'loct1');
postgres=# alter table itrtest attach partition remp1 for values in (1);

This behaves oddly:

postgres=# insert into itrtest values (1, 'foo') returning *;
 a |  b
---+-----
 1 | foo
(1 row)

INSERT 0 1

The new value of b in the RETURNING result should be concatenated with '
triggered !', as shown below:

postgres=# select tableoid::regclass, * from itrtest ;
 tableoid | a |        b
----------+---+-----------------
 remp1    | 1 | foo triggered !
(1 row)

The reason for that is: since that in ExecInitPartitionInfo, the core
calls BeginForeignInsert via ExecInitRoutingInfo before initializing
ri_returningList of ResultRelInfo for the partition, BeginForeignInsert
sees that the ri_returningList is NULL and incorrectly recognizes that
the local query doesn't have a RETURNING list.  So, I moved the
ExecInitRoutingInfo call after building the ri_returningList (and before
handling ON CONFLICT because we reference the conversion map created by
that when handling that clause).  The first version of the patch called
BeginForeignInsert that order, but I updated the patch incorrectly.  :(
 Also, I removed the CheckValidResultRel check from ExecInitRoutingInfo
and added that to ExecInitPartitionInfo, right after the
InitResultRelInfo call, because it would be better to abort the
operation as soon as we find the partition to be non-routable.  Another
thing I noticed is the RT index of the foreign partition set to 1 in
postgresBeginForeignInsert to create a remote query; that would not work
for cases where the partitioned table has an RT index larger than 1 (eg,
the partitioned table is not the primary ModifyTable node); in which
cases, since the varno of Vars belonging to the foreign partition in the
RETURNING expressions is the same as the partitioned table, calling
deparseReturningList with the RT index set to 1 against the RETURNING
expressions would produce attrs_used to be NULL, leading to postgres_fdw
not retrieving actually inserted data from the remote, even if remote
triggers might change those data.  So, I fixed this as well, by setting
the RT index accordingly to the partitioned table.  Attached is a patch
for fixing these issues.  I'll add this to the open items list for PG11.
 (After addressing this, I'll post an updated version of the
fix-postgres_fdw-WCO-handling patch.)

Best regards,
Etsuro Fujita