From 4deb57de7dcfb66f850f16c0324731fdf3d86d09 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Sat, 26 Jan 2013 13:33:24 -0500 Subject: [PATCH] Issue ERROR if FREEZE mode can't be honored by COPY Previously non-honored FREEZE mode was ignored. This also issues an appropriate error message based on the cause of the failure, per suggestion from Tom. Additional regression test case added. --- doc/src/sgml/ref/copy.sgml | 12 ++++------- src/backend/commands/copy.c | 32 ++++++++++++++++++----------- src/test/regress/expected/copy2.out | 18 ++++++++-------- src/test/regress/sql/copy2.sql | 9 +++++++- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml index 6a0fabc978..2137c67cb4 100644 --- a/doc/src/sgml/ref/copy.sgml +++ b/doc/src/sgml/ref/copy.sgml @@ -190,18 +190,14 @@ COPY { table_name [ ( VACUUM FREEZE command. This is intended as a performance option for initial data loading. Rows will be frozen only if the table being loaded has been created - in the current subtransaction, there are no cursors open and there - are no older snapshots held by this transaction. If those conditions - are not met the command will continue without error though will not - freeze rows. It is also possible in rare cases that the request - cannot be honoured for internal reasons, hence FREEZE - is more of a guideline than a hard rule. + or truncated in the current subtransaction, there are no cursors + open and there are no older snapshots held by this transaction. Note that all other sessions will immediately be able to see the data once it has been successfully loaded. This violates the normal rules - of MVCC visibility and by specifying this option the user acknowledges - explicitly that this is understood. + of MVCC visibility and users specifying should be aware of the + potential problems this might cause. diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 8778e8b15c..49cc8dd88f 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -1978,8 +1978,6 @@ CopyFrom(CopyState cstate) * ROLLBACK TO save; * COPY ... * - * However this is OK since at worst we will fail to make the optimization. - * * Also, if the target file is new-in-transaction, we assume that checking * FSM for free space is a waste of time, even if we must use WAL because * of archiving. This could possibly be wrong, but it's unlikely. @@ -1991,6 +1989,7 @@ CopyFrom(CopyState cstate) * no additional work to enforce that. *---------- */ + /* createSubid is creation check, newRelfilenodeSubid is truncation check */ if (cstate->rel->rd_createSubid != InvalidSubTransactionId || cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId) { @@ -2006,18 +2005,27 @@ CopyFrom(CopyState cstate) * after xact cleanup. Note that the stronger test of exactly * which subtransaction created it is crucial for correctness * of this optimisation. - * - * As noted above rd_newRelfilenodeSubid is not set in all cases - * where we can apply the optimization, so in those rare cases - * where we cannot honour the request we do so silently. */ - if (cstate->freeze && - ThereAreNoPriorRegisteredSnapshots() && - ThereAreNoReadyPortals() && - (cstate->rel->rd_newRelfilenodeSubid == GetCurrentSubTransactionId() || - cstate->rel->rd_createSubid == GetCurrentSubTransactionId())) - hi_options |= HEAP_INSERT_FROZEN; + if (cstate->freeze) + { + if (!ThereAreNoPriorRegisteredSnapshots() || !ThereAreNoReadyPortals()) + ereport(ERROR, + (ERRCODE_INVALID_TRANSACTION_STATE, + errmsg("cannot perform FREEZE because of prior transaction activity"))); + + if (cstate->rel->rd_createSubid == GetCurrentSubTransactionId() || + cstate->rel->rd_newRelfilenodeSubid == GetCurrentSubTransactionId()) + hi_options |= HEAP_INSERT_FROZEN; + else + ereport(ERROR, + (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE, + errmsg("cannot perform FREEZE because of transaction activity after table creation or truncation"))); + } } + else if (cstate->freeze) + ereport(ERROR, + (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE, + errmsg("cannot perform FREEZE because the table was not created or truncated in the current transaction"))); /* * We need a ResultRelInfo so we can use the regular executor's diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out index 78c601fd9b..5777b24274 100644 --- a/src/test/regress/expected/copy2.out +++ b/src/test/regress/expected/copy2.out @@ -334,22 +334,20 @@ SELECT * FROM vistest; COMMIT; TRUNCATE vistest; COPY vistest FROM stdin CSV FREEZE; +ERROR: cannot perform FREEZE because the table was not created or truncated in the current transaction +BEGIN; +TRUNCATE vistest; +SAVEPOINT s1; +COPY vistest FROM stdin CSV FREEZE; +ERROR: cannot perform FREEZE because of transaction activity after table creation or truncation +COMMIT; BEGIN; INSERT INTO vistest VALUES ('z'); SAVEPOINT s1; TRUNCATE vistest; ROLLBACK TO SAVEPOINT s1; COPY vistest FROM stdin CSV FREEZE; -SELECT * FROM vistest; - a ----- - p - g - z - d3 - e -(5 rows) - +ERROR: cannot perform FREEZE because the table was not created or truncated in the current transaction COMMIT; CREATE FUNCTION truncate_in_subxact() RETURNS VOID AS $$ diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql index 55568e68e4..c46128b38a 100644 --- a/src/test/regress/sql/copy2.sql +++ b/src/test/regress/sql/copy2.sql @@ -234,6 +234,14 @@ p g \. BEGIN; +TRUNCATE vistest; +SAVEPOINT s1; +COPY vistest FROM stdin CSV FREEZE; +m +k +\. +COMMIT; +BEGIN; INSERT INTO vistest VALUES ('z'); SAVEPOINT s1; TRUNCATE vistest; @@ -242,7 +250,6 @@ COPY vistest FROM stdin CSV FREEZE; d3 e \. -SELECT * FROM vistest; COMMIT; CREATE FUNCTION truncate_in_subxact() RETURNS VOID AS $$