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.

diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 6a0fabc978de96dab22381c24e9fcbe7b0e0f187..2137c67cb4b542344017b08c26cf4a36d894a1c1 100644 (file)
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -190,18 +190,14 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
       would be after running the <command>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 <literal>FREEZE</literal>
-      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.
      </para>
      <para>
       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.
      </para>
     </listitem>
    </varlistentry>

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 8778e8b15c3da93255f7a5f7a4b34445fd94157f..49cc8dd88f4550f2bac73ad2d7d18bb80b078ee0 100644 (file)
--- 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 78c601fd9ba99ab17294bc10f7fc4bd615251425..5777b242747d2ef91cb84fae5ac8d4546fd17573 100644 (file)
--- 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 55568e68e406b30797f91c21c924ee967b3e6c64..c46128b38a1738a62994a329a59a4d420925bd2e 100644 (file)
--- 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
 $$