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

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_level need 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