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:23 +0000 (19:54 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Mar 2008 19:54:23 +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 e3a898731cd481949a2a0195683ccd4cb6527511..f468daae4036dfc5775ba77690a932a38034e1be 100644 (file)
@@ -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 5be261159c30c6bfc5a7cfc1d56ae656e6b53b26..192b8add0121564fef44d9a1ef4d60b30d9b0d9b 100644 (file)
@@ -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 e216363b9836479549e8f06bcef7312c9808abe4..9d378279d41c29022c643cd7a11e8882a96a1c8e 100644 (file)
 #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 20c284d2fd7fef3d1cc80ec1083d99c81ca7c981..2f821d0b1220e658f5edd42dbc47525ecdbe94e3 100644 (file)
@@ -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 56e5c6fa1503476a7c1637cf22a38676413ee044..660f81dec861b922b3faf65a9f7e89a2d65ab4a8 100644 (file)
@@ -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 55301e4cae0bd560043664ffcc8ae7d9ed98f58d..42a890eb9ddfea62d5abc969af21122bf2efd578 100644 (file)
@@ -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);