Issue ERROR if FREEZE mode can't be honored by COPY
authorBruce Momjian
Sat, 26 Jan 2013 18:33:24 +0000 (13:33 -0500)
committerBruce Momjian
Sat, 26 Jan 2013 18:33:24 +0000 (13:33 -0500)
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
src/backend/commands/copy.c
src/test/regress/expected/copy2.out
src/test/regress/sql/copy2.sql

index 6a0fabc978de96dab22381c24e9fcbe7b0e0f187..2137c67cb4b542344017b08c26cf4a36d894a1c1 100644 (file)
@@ -190,18 +190,14 @@ COPY { table_name [ ( 
       would be after running the 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.
      
     
    
index 8778e8b15c3da93255f7a5f7a4b34445fd94157f..49cc8dd88f4550f2bac73ad2d7d18bb80b078ee0 100644 (file)
@@ -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
index 78c601fd9ba99ab17294bc10f7fc4bd615251425..5777b242747d2ef91cb84fae5ac8d4546fd17573 100644 (file)
@@ -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
 $$
index 55568e68e406b30797f91c21c924ee967b3e6c64..c46128b38a1738a62994a329a59a4d420925bd2e 100644 (file)
@@ -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
 $$