Re: BUG #15552: Unexpected error in COPY to a foreign table in atransaction

Hi,

On 2018/12/17 22:12, Luis Carril wrote:
> Hi,
> 
>> heap_sync should only be called for relations that actually have files to
>> sync, which isn't true for foreign tables.  So, a simple relkind check
>> before calling heap_sync() in CopyFrom would suffice I think.  Although,
>> we might also need such a check higher up in CopyFrom where some
>> optimizations that are specific to "heap" relations are enabled.  For
>> example, this piece of code:
> 
>   thanks for the input, so it seems that is enough with adding the check as you suggested:
> 
>     if (cstate->rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE &&
>         cstate->rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE &&
>         (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
>          cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId))
>     {
>         hi_options |= HEAP_INSERT_SKIP_FSM;
>         if (!XLogIsNeeded())
>             hi_options |= HEAP_INSERT_SKIP_WAL;
>     }

I think that would do the trick.

>    Should a regression test be added (at postgres_fdw.sql)? If yes, how should be implemented, as max_wal_senders and
wal_levelneed to be changed (to 0 and minimal) to trigger the bug, which is only possible on server start.
 

I noticed that too.  As you say, it is not possible to exercise a test we
might add for this with `make check`, because it runs with a fixed value
of wal_level (= replica).  But it is possible with `make installcheck` on
a cluster that's running with wal_level = minimal.  Maybe, something like
this:

+ -- Test that COPY FROM works correctly when the foreign table is created in
+ -- the same transaction
+ create table test_copy_same_txn_loc (f1 int, f2 text);
+ alter table test_copy_same_txn_loc set (autovacuum_enabled = 'false');
+ begin;
+ create foreign table test_copy_same_txn_rem (f1 int, f2 text) server
loopback options(table_name 'test_copy_same_txn_loc');
+ copy test_copy_same_txn_rem from stdin;
+ ERROR:  could not open file "base/19888/20234": No such file or directory
+ commit;
+ drop table test_copy_same_txn_loc;
+ drop foreign table test_copy_same_txn_rem;
+ ERROR:  foreign table "test_copy_same_txn_rem" does not exist
  -- ===================================================================
  -- test IMPORT FOREIGN SCHEMA
  -- ===================================================================

From the above output, your patch will make "ERROR: could not open file
xxx" go away.

Thanks,
Amit