Fix incorrect ordering of smgr cleanup relative to buffer pin cleanup

during transaction abort.  Add a regression test case to catch related
mistakes in future.  Alvaro Herrera and Tom Lane.

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index cc36548b1814aadf68aa659065e5caaa0d38cedc..92c9de0ea755182c7c0e482a540613417e6233f0 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.185 2004/08/30 19:00:03 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.186 2004/09/06 17:56:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1333,9 +1333,6 @@ CommitTransaction(void)
         * backend-wide state.
         */
 
-       smgrDoPendingDeletes(true);
-       /* smgrcommit already done */
-
        CallXactCallbacks(XACT_EVENT_COMMIT, InvalidTransactionId);
 
        ResourceOwnerRelease(TopTransactionResourceOwner,
@@ -1352,6 +1349,14 @@ CommitTransaction(void)
         */
        AtEOXact_Inval(true);
 
+       /*
+        * Likewise, dropping of files deleted during the transaction is best done
+        * after releasing relcache and buffer pins.  (This is not strictly
+        * necessary during commit, since such pins should have been released
+        * already, but this ordering is definitely critical during abort.)
+        */
+       smgrDoPendingDeletes(true);
+
        ResourceOwnerRelease(TopTransactionResourceOwner,
                                                 RESOURCE_RELEASE_LOCKS,
                                                 true, true);
@@ -1363,6 +1368,7 @@ CommitTransaction(void)
        AtEOXact_SPI(true);
        AtEOXact_on_commit_actions(true, s->transactionIdData);
        AtEOXact_Namespace(true);
+       /* smgrcommit already done */
        AtEOXact_Files();
        pgstat_count_xact_commit();
 
@@ -1481,15 +1487,13 @@ AbortTransaction(void)
         * ordering.
         */
 
-       smgrDoPendingDeletes(false);
-       smgrabort();
-
        CallXactCallbacks(XACT_EVENT_ABORT, InvalidTransactionId);
 
        ResourceOwnerRelease(TopTransactionResourceOwner,
                                                 RESOURCE_RELEASE_BEFORE_LOCKS,
                                                 false, true);
        AtEOXact_Inval(false);
+       smgrDoPendingDeletes(false);
        ResourceOwnerRelease(TopTransactionResourceOwner,
                                                 RESOURCE_RELEASE_LOCKS,
                                                 false, true);
@@ -1501,6 +1505,7 @@ AbortTransaction(void)
        AtEOXact_SPI(false);
        AtEOXact_on_commit_actions(false, s->transactionIdData);
        AtEOXact_Namespace(false);
+       smgrabort();
        AtEOXact_Files();
        pgstat_count_xact_rollback();
 
@@ -3014,7 +3019,6 @@ CommitSubTransaction(void)
        AtSubCommit_Notify();
        AtEOSubXact_UpdatePasswordFile(true, s->transactionIdData,
                                                                   s->parent->transactionIdData);
-       AtSubCommit_smgr();
 
        CallXactCallbacks(XACT_EVENT_COMMIT_SUB, s->parent->transactionIdData);
 
@@ -3024,6 +3028,7 @@ CommitSubTransaction(void)
        AtEOSubXact_RelationCache(true, s->transactionIdData,
                                                          s->parent->transactionIdData);
        AtEOSubXact_Inval(true);
+       AtSubCommit_smgr();
        ResourceOwnerRelease(s->curTransactionOwner,
                                                 RESOURCE_RELEASE_LOCKS,
                                                 true, false);
@@ -3109,8 +3114,6 @@ AbortSubTransaction(void)
        RecordSubTransactionAbort();
 
        /* Post-abort cleanup */
-       AtSubAbort_smgr();
-
        CallXactCallbacks(XACT_EVENT_ABORT_SUB, s->parent->transactionIdData);
 
        ResourceOwnerRelease(s->curTransactionOwner,
@@ -3119,6 +3122,7 @@ AbortSubTransaction(void)
        AtEOSubXact_RelationCache(false, s->transactionIdData,
                                                          s->parent->transactionIdData);
        AtEOSubXact_Inval(false);
+       AtSubAbort_smgr();
        ResourceOwnerRelease(s->curTransactionOwner,
                                                 RESOURCE_RELEASE_LOCKS,
                                                 false, false);

diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 71e82d71edeebdc3f384bc635d43333ed42864a4..80b8c9f74a75d05e589c5f991c22894a7ff5379b 100644 (file)
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/smgr/smgr.c,v 1.81 2004/08/30 02:54:39 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/smgr/smgr.c,v 1.82 2004/09/06 17:56:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -784,7 +784,7 @@ smgrcommit(void)
 }
 
 /*
- *     smgrabort() -- Abort changes made during the current transaction.
+ *     smgrabort() -- Clean up after transaction abort.
  */
 void
 smgrabort(void)

diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index 2b8470c931d05740506021495e18c45b6468d256..955558d690b24088b95424703511d956a29d929f 100644 (file)
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -374,6 +374,21 @@ ERROR:  portal "c" cannot be run
        FETCH 10 FROM c;
 ERROR:  portal "c" cannot be run
 COMMIT;
+-- test case for problems with dropping an open relation during abort
+BEGIN;
+       savepoint x;
+               CREATE TABLE koju (a INT UNIQUE);
+NOTICE:  CREATE TABLE / UNIQUE will create implicit index "koju_a_key" for table "koju"
+               INSERT INTO koju VALUES (1);
+               INSERT INTO koju VALUES (1);
+ERROR:  duplicate key violates unique constraint "koju_a_key"
+       rollback to x;
+       CREATE TABLE koju (a INT UNIQUE);
+NOTICE:  CREATE TABLE / UNIQUE will create implicit index "koju_a_key" for table "koju"
+       INSERT INTO koju VALUES (1);
+       INSERT INTO koju VALUES (1);
+ERROR:  duplicate key violates unique constraint "koju_a_key"
+ROLLBACK;
 DROP TABLE foo;
 DROP TABLE baz;
 DROP TABLE barbaz;

diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index d83a9f077fa1b7f16264883b13abbbe43f071960..81006d16d23d381faa73e188058674a7893490c2 100644 (file)
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -231,6 +231,19 @@ BEGIN;
        FETCH 10 FROM c;
 COMMIT;
 
+-- test case for problems with dropping an open relation during abort
+BEGIN;
+       savepoint x;
+               CREATE TABLE koju (a INT UNIQUE);
+               INSERT INTO koju VALUES (1);
+               INSERT INTO koju VALUES (1);
+       rollback to x;
+
+       CREATE TABLE koju (a INT UNIQUE);
+       INSERT INTO koju VALUES (1);
+       INSERT INTO koju VALUES (1);
+ROLLBACK;
+
 DROP TABLE foo;
 DROP TABLE baz;
 DROP TABLE barbaz;