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

On Wed, Dec 19, 2018 at 10:19:58AM +0900, Amit Langote wrote:
> Not sure that's a good name to give to the test file, although since I'm
> not greatly enthusiastic about adding a TAP test for this, I don't have a
> good suggestion.  Maybe, choose a more generic name?  Michael, any
> suggestions?

I have looked at the proposal seriously myself, and adding a TAP test
for that edge case is overdoing it.  So why not just adding a test to
postgres_fdw.sql?  The failure can be triggered with installcheck and
wal_level =3D minimal.  Not everybody tests with non-default options, but
some of us do.

> @@ -2401,6 +2401,7 @@ CopyFrom(CopyState cstate)
>  ^I */
>  ^I/* createSubid is creation check, newRelfilenodeSubid is truncation
> check */
>  ^Iif (cstate->rel->rd_rel->relkind !=3D RELKIND_PARTITIONED_TABLE &&
> +^I    cstate->rel->rd_rel->relkind !=3D RELKIND_FOREIGN_TABLE &&
>  ^I^I(cstate->rel->rd_createSubid !=3D InvalidSubTransactionId ||
>  ^I^I cstate->rel->rd_newRelfilenodeSubid !=3D InvalidSubTransactionId))
>  ^I{
>=20
> It seems you've used spaces to indent the newly added line.  Please,
> change that to a tab like neighboring lines.

Indeed.

Attached is a more simple, updated, patch which adds a new test and a
comment.  I would rather not play with the semantics of heap_sync() on
the back branches as well as on a thread dealing with a bug about COPY
with foreign tables.  Such discussions deserve a larger audience on
-hackers.

it is a bit funny to see COPY FREEZE working for foreign tables
actually, but perhaps this has some use-cases for some FDWs, so I'd
rather not touch it.
--
Michael