From: Michael Paquier <michael@paquier.xyz>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Luis Carril <luis.carril@swarm64.com>
Cc: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Cc: PG Bug reporting form <noreply@postgresql.org>
Cc: Michael Paquier <michael.paquier@gmail.com>
Subject: Re: BUG #15552: Unexpected error in COPY to a foreign table in a transaction
Date: Wed, 19 Dec 2018 11:38:14 +0900
Message-ID: <20181219023814.GA6577@paquier.xyz> (raw)
In-Reply-To: <3539e73a-313a-5406-27c7-850affb47921@lab.ntt.co.jp>
References: <15552-c64aa14c5c22f63c@postgresql.org>
<8e82d10d-cc16-eed4-7634-3654f59d8053@lab.ntt.co.jp>
<LEJPR01MB00109D0BECB8A961074A7CAFE7BC0@LEJPR01MB0010.DEUPRD01.PROD.OUTLOOK.DE>
<30718b48-8020-329a-c6f0-74e1222e5283@lab.ntt.co.jp>
<5C18ECBB.2000507@lab.ntt.co.jp>
<LEJPR01MB001074FD67CB65B539122E4DE7BD0@LEJPR01MB0010.DEUPRD01.PROD.OUTLOOK.DE>
<3539e73a-313a-5406-27c7-850affb47921@lab.ntt.co.jp>
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
Attachments:
[text/x-diff] no_heap_opt_fdw_copy-v4.patch (2.6K, ../20181219023814.GA6577@paquier.xyz/2-no_heap_opt_fdw_copy-v4.patch)
download | inline diff:diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.outindex 666548ba69..bb92d9d37a 100644--- a/contrib/postgres_fdw/expected/postgres_fdw.out+++ b/contrib/postgres_fdw/expected/postgres_fdw.out@@ -7981,6 +7981,22 @@ drop trigger rem2_trig_row_before on rem2;
drop trigger rem2_trig_row_after on rem2;
drop trigger loc2_trig_row_before_insert on loc2;
delete from rem2;
+-- test COPY FROM with foreign table created in the same transaction+create table loc3 (f1 int, f2 text);+begin;+create foreign table rem3 (f1 int, f2 text)+ server loopback options(table_name 'loc3');+copy rem3 from stdin;+commit;+select * from rem3;+ f1 | f2 +----+-----+ 1 | foo+ 2 | bar+(2 rows)++drop foreign table rem3;+drop table loc3;
-- ===================================================================
-- test IMPORT FOREIGN SCHEMA
-- ===================================================================
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sqlindex f963e99ab6..f438165650 100644--- a/contrib/postgres_fdw/sql/postgres_fdw.sql+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql@@ -2124,6 +2124,20 @@ drop trigger loc2_trig_row_before_insert on loc2;
delete from rem2;
+-- test COPY FROM with foreign table created in the same transaction+create table loc3 (f1 int, f2 text);+begin;+create foreign table rem3 (f1 int, f2 text)+ server loopback options(table_name 'loc3');+copy rem3 from stdin;+1 foo+2 bar+\.+commit;+select * from rem3;+drop foreign table rem3;+drop table loc3;+
-- ===================================================================
-- test IMPORT FOREIGN SCHEMA
-- ===================================================================
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.cindex 4311e16007..4d39509c61 100644--- a/src/backend/commands/copy.c+++ b/src/backend/commands/copy.c@@ -2397,10 +2397,14 @@ CopyFrom(CopyState cstate)
* possible to improve on this, but it does mean maintaining heap insert
* option flags per partition and setting them when we first open the
* partition.
+ *+ * This optimization is not supported for foreign tables as they have no+ * physical storage.
*----------
*/
/* createSubid is creation check, newRelfilenodeSubid is truncation check */
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))
{
[application/pgp-signature] signature.asc (832B, ../20181219023814.GA6577@paquier.xyz/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, Langote_Amit_f8@lab.ntt.co.jp, luis.carril@swarm64.com, fujita.etsuro@lab.ntt.co.jp, pgsql-bugs@lists.postgresql.org, noreply@postgresql.org, michael.paquier@gmail.com
Subject: Re: BUG #15552: Unexpected error in COPY to a foreign table in a transaction
In-Reply-To: <20181219023814.GA6577@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox