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

Hi Luis and Amit,

Thanks for the report, Luis!  Thanks for the discussion, Amit!

(2018/12/18 12:24), Amit Langote wrote:
> On 2018/12/17 22:12, Luis Carril wrote:
>>> 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.

I think so too.

FDWs would not look at heap_insert options, so another option would be 
to 1) leave that options as-is for foreign tables and 2) if the target 
is a foreign table, just skip heap_sync at the bottom of CopyFrom, or 
just return without doing anything in heap_sync.

Best regards,
Etsuro Fujita