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