Re: postgres_fdw: batch inserts vs. before row triggers

Hi,

While working on something else, I notice some more oddities.  Here is
an example:

create extension postgres_fdw;
create server loopback foreign data wrapper postgres_fdw options
(dbname 'postgres');
create user mapping for current_user server loopback;
create table t1 (a text, b int);
create foreign table ft1 (a text, b int) server loopback options
(table_name 't1');
create table w1 (a text, b int);
create function ft1_rowcount_trigf() returns trigger language plpgsql
as $$ begin raise notice '%: there are % rows in ft1', tg_name,
(select count(*) from ft1); return new; end; $$;
create trigger ft1_rowcount_trigger before insert on w1 for each row
execute function ft1_rowcount_trigf();
alter server loopback options (add batch_size '10');

with t as (insert into w1 values ('foo', 10), ('bar', 20) returning *)
insert into ft1 select * from t;
NOTICE:  ft1_rowcount_trigger: there are 0 rows in ft1
NOTICE:  ft1_rowcount_trigger: there are 0 rows in ft1
INSERT 0 2

The command tag shows that two rows were inserted into ft1, but:

select * from ft1;
  a  | b
-----+----
 foo | 10
 bar | 20
 foo | 10
 bar | 20
(4 rows)

ft1 has four rows, which is wrong.  Also, when inserting the second
row (‘bar’, 20) into w1, the BEFORE ROW INSERT trigger should see the
first row (‘foo’, 10) in ft1, but it reports no rows were visible
there.

The reason for the former is that this bit added by commit b663a4136
is done not only when running the primary ModifyTable node but when
running the secondary ModifyTable node (with the wrong
ModifyTableState).

    /*
     * Insert remaining tuples for batch insert.
     */
    if (proute)
        relinfos = estate->es_tuple_routing_result_relations;
    else
        relinfos = estate->es_opened_result_relations;

    foreach(lc, relinfos)
    {
        resultRelInfo = lfirst(lc);
        if (resultRelInfo->ri_NumSlots > 0)
            ExecBatchInsert(node, resultRelInfo,
                            resultRelInfo->ri_Slots,
                            resultRelInfo->ri_PlanSlots,
                            resultRelInfo->ri_NumSlots,
                            estate, node->canSetTag);
    }

The reason for the latter is that that commit fails to flush pending
inserts before executing any BEFORE ROW triggers, so that rows are
visible to such triggers.

Attached is a patch for fixing these issues.  In the patch I added to
the EState struct a List member es_insert_pending_result_relations to
store ResultRelInfos for foreign tables on which batch inserts are to
be performed, so that we avoid scanning through
es_tuple_routing_result_relations or es_opened_result_relations each
time when flushing pending inserts to the foreign tables.

Best regards,
Etsuro Fujita