Fix PREPARE TRANSACTION to reject the case where the transaction has dropped a
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Mar 2008 19:54:06 +0000 (19:54 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Mar 2008 19:54:06 +0000 (19:54 +0000)
temporary table; we can't support that because there's no way to clean up the
source backend's internal state if the eventual COMMIT PREPARED is done by
another backend.  This was checked correctly in 8.1 but I broke it in 8.2 :-(.
Patch by Heikki Linnakangas, original trouble report by John Smith.

src/backend/access/heap/heapam.c
src/backend/access/transam/xact.c
src/backend/storage/lmgr/lmgr.c
src/backend/storage/lmgr/lock.c
src/include/access/xact.h
src/include/storage/lmgr.h

index c3a45b98db5bda0a96cf00cba0fa0d0fab0048c5..166fed7233c70470b0807507fa55602c294ab3d2 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.249 2008/01/30 18:35:55 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.250 2008/03/04 19:54:06 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -868,6 +868,10 @@ relation_open(Oid relationId, LOCKMODE lockmode)
    if (!RelationIsValid(r))
        elog(ERROR, "could not open relation with OID %u", relationId);
 
+   /* Make note that we've accessed a temporary relation */
+   if (r->rd_istemp)
+       MyXactAccessedTempRel = true;
+
    pgstat_initstats(r);
 
    return r;
@@ -912,6 +916,10 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
    if (!RelationIsValid(r))
        elog(ERROR, "could not open relation with OID %u", relationId);
 
+   /* Make note that we've accessed a temporary relation */
+   if (r->rd_istemp)
+       MyXactAccessedTempRel = true;
+
    pgstat_initstats(r);
 
    return r;
@@ -958,6 +966,10 @@ relation_open_nowait(Oid relationId, LOCKMODE lockmode)
    if (!RelationIsValid(r))
        elog(ERROR, "could not open relation with OID %u", relationId);
 
+   /* Make note that we've accessed a temporary relation */
+   if (r->rd_istemp)
+       MyXactAccessedTempRel = true;
+
    pgstat_initstats(r);
 
    return r;
index 13c3a0378b143a8b7d9e2044f49fc712b692df61..65611089c417031601aa1b9191f5da250f3a40ee 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.257 2008/01/15 18:56:59 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.258 2008/03/04 19:54:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -62,6 +62,13 @@ bool     XactSyncCommit = true;
 int            CommitDelay = 0;    /* precommit delay in microseconds */
 int            CommitSiblings = 5; /* # concurrent xacts needed to sleep */
 
+/*
+ * MyXactAccessedTempRel is set when a temporary relation is accessed.
+ * We don't allow PREPARE TRANSACTION in that case.  (This is global
+ * so that it can be set from heapam.c.)
+ */
+bool       MyXactAccessedTempRel = false;
+
 
 /*
  * transaction states - transaction state from server perspective
@@ -1445,6 +1452,7 @@ StartTransaction(void)
    XactIsoLevel = DefaultXactIsoLevel;
    XactReadOnly = DefaultXactReadOnly;
    forceSyncCommit = false;
+   MyXactAccessedTempRel = false;
 
    /*
     * reinitialize within-transaction counters
@@ -1770,6 +1778,26 @@ PrepareTransaction(void)
 
    /* NOTIFY and flatfiles will be handled below */
 
+   /*
+    * Don't allow PREPARE TRANSACTION if we've accessed a temporary table
+    * in this transaction.  Having the prepared xact hold locks on another
+    * backend's temp table seems a bad idea --- for instance it would prevent
+    * the backend from exiting.  There are other problems too, such as how
+    * to clean up the source backend's local buffers and ON COMMIT state
+    * if the prepared xact includes a DROP of a temp table.
+    *
+    * We must check this after executing any ON COMMIT actions, because
+    * they might still access a temp relation.
+    *
+    * XXX In principle this could be relaxed to allow some useful special
+    * cases, such as a temp table created and dropped all within the
+    * transaction.  That seems to require much more bookkeeping though.
+    */
+   if (MyXactAccessedTempRel)
+       ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+
    /* Prevent cancel/die interrupt while cleaning up */
    HOLD_INTERRUPTS();
 
index a882f4e432f354ed921c26af586bc9e1a6587a9a..67bb2ba9fb292b9d3dc0d5c12caf5ff6805cbd05 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/lmgr/lmgr.c,v 1.96 2008/01/08 23:18:50 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/lmgr/lmgr.c,v 1.97 2008/03/04 19:54:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "access/transam.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
-#include "catalog/namespace.h"
 #include "miscadmin.h"
 #include "storage/lmgr.h"
 #include "storage/procarray.h"
 #include "utils/inval.h"
-#include "utils/lsyscache.h"
 
 
 /*
@@ -663,44 +661,6 @@ UnlockSharedObject(Oid classid, Oid objid, uint16 objsubid,
 }
 
 
-/*
- * LockTagIsTemp
- *     Determine whether a locktag is for a lock on a temporary object
- *
- * We need this because 2PC cannot deal with temp objects
- */
-bool
-LockTagIsTemp(const LOCKTAG *tag)
-{
-   switch ((LockTagType) tag->locktag_type)
-   {
-       case LOCKTAG_RELATION:
-       case LOCKTAG_RELATION_EXTEND:
-       case LOCKTAG_PAGE:
-       case LOCKTAG_TUPLE:
-           /* check for lock on a temp relation */
-           /* field1 is dboid, field2 is reloid for all of these */
-           if ((Oid) tag->locktag_field1 == InvalidOid)
-               return false;   /* shared, so not temp */
-           if (isTempOrToastNamespace(get_rel_namespace((Oid) tag->locktag_field2)))
-               return true;
-           break;
-       case LOCKTAG_TRANSACTION:
-       case LOCKTAG_VIRTUALTRANSACTION:
-           /* there are no temp transactions */
-           break;
-       case LOCKTAG_OBJECT:
-           /* there are currently no non-table temp objects */
-           break;
-       case LOCKTAG_USERLOCK:
-       case LOCKTAG_ADVISORY:
-           /* assume these aren't temp */
-           break;
-   }
-   return false;               /* default case */
-}
-
-
 /*
  * Append a description of a lockable object to buf.
  *
index 24f9b7782ed2817e3c325e0449bf7764a77373f5..c8ba3538cf581f6f1ee5bc784336c25f33d9796f 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.181 2008/02/02 22:26:17 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.182 2008/03/04 19:54:06 tgl Exp $
  *
  * NOTES
  *   A lock table is a shared memory hash table.  When
@@ -37,7 +37,6 @@
 #include "access/twophase_rmgr.h"
 #include "miscadmin.h"
 #include "pgstat.h"
-#include "storage/lmgr.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/resowner.h"
@@ -1871,12 +1870,6 @@ AtPrepare_Locks(void)
                elog(ERROR, "cannot PREPARE when session locks exist");
        }
 
-       /* Can't handle it if the lock is on a temporary object */
-       if (LockTagIsTemp(&locallock->tag.lock))
-           ereport(ERROR,
-                   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                    errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
-
        /*
         * Create a 2PC record.
         */
index c0020c8bcd50b09e610dd3426c3d5398b4e4d4a1..dff05c73db58a579010424d7c6d2796a75d0fb39 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/xact.h,v 1.93 2008/01/01 19:45:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/xact.h,v 1.94 2008/03/04 19:54:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,6 +44,9 @@ extern bool XactReadOnly;
 /* Asynchronous commits */
 extern bool XactSyncCommit;
 
+/* Kluge for 2PC support */
+extern bool MyXactAccessedTempRel;
+
 /*
  * start- and end-of-transaction callbacks for dynamically loaded modules
  */
index b89d0c6c8d9b617d8813e321a1c4a11451356e22..05150afbac02bb251828640b93e341a8e55bb81c 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/lmgr.h,v 1.60 2008/01/01 19:45:59 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/storage/lmgr.h,v 1.61 2008/03/04 19:54:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -72,9 +72,6 @@ extern void LockSharedObject(Oid classid, Oid objid, uint16 objsubid,
 extern void UnlockSharedObject(Oid classid, Oid objid, uint16 objsubid,
                   LOCKMODE lockmode);
 
-/* Knowledge about which locktags describe temp objects */
-extern bool LockTagIsTemp(const LOCKTAG *tag);
-
 /* Describe a locktag for error messages */
 extern void DescribeLockTag(StringInfo buf, const LOCKTAG *tag);