Transactions and temp tables - Mailing list pgsql-hackers

Hi,

I had the same problem as John with "could not open relation
1663/16384/16584: No such file or directory" in a specific combination
of transactions with temp tables
(http://archives.postgresql.org/pgsql-hackers/2008-02/msg01260.php). As
Heikki mentioned
(http://archives.postgresql.org/pgsql-hackers/2008-02/msg01277.php) we
should be able to allow CREATE+DROP in the same transaction.

I came up with a patch (currently based on 8.3.3) to address that issue.
Instead of relying on a boolean that tells if a temp table was accessed,
I keep a list of the Oid for the temp tables accessed in the transaction
and at prepare commit time, I check if the relations are still valid. I
also added a check to allow empty temp tables at prepare commit time
(this allows to use temp tables with 'on commit delete rows' options.

I am attaching the patch and the use cases I have been using to test it.
The test cases try to compile the various use cases that I have seen
reported on the list. Let me know what you think of the patch and if it
could be applied to 8.3 and 8.4?

Thanks in advance for your feedback,
manu

--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet

### Eclipse Workspace Patch 1.0
#P Postgres-8.3.3
Index: src/include/access/xact.h
===================================================================
RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v
retrieving revision 1.93.2.1
diff -u -r1.93.2.1 xact.h
--- src/include/access/xact.h    4 Mar 2008 19:54:13 -0000    1.93.2.1
+++ src/include/access/xact.h    6 Oct 2008 20:39:46 -0000
@@ -18,6 +18,7 @@
 #include "nodes/pg_list.h"
 #include "storage/relfilenode.h"
 #include "utils/timestamp.h"
+#include "postgres_ext.h"


 /*
@@ -44,8 +45,8 @@
 /* Asynchronous commits */
 extern bool XactSyncCommit;

-/* Kluge for 2PC support */
-extern bool MyXactAccessedTempRel;
+/* List of temp tables accessed during a transaction for 2PC support */
+extern void enlistRelationIdFor2PCChecks(Oid relationId);

 /*
  *    start- and end-of-transaction callbacks for dynamically loaded modules
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.249.2.2
diff -u -r1.249.2.2 heapam.c
--- src/backend/access/heap/heapam.c    8 Mar 2008 21:58:07 -0000    1.249.2.2
+++ src/backend/access/heap/heapam.c    6 Oct 2008 20:39:46 -0000
@@ -870,7 +870,7 @@

     /* Make note that we've accessed a temporary relation */
     if (r->rd_istemp)
-        MyXactAccessedTempRel = true;
+        enlistRelationIdFor2PCChecks(relationId);

     pgstat_initstats(r);

@@ -918,7 +918,7 @@

     /* Make note that we've accessed a temporary relation */
     if (r->rd_istemp)
-        MyXactAccessedTempRel = true;
+        enlistRelationIdFor2PCChecks(relationId);

     pgstat_initstats(r);

@@ -968,7 +968,7 @@

     /* Make note that we've accessed a temporary relation */
     if (r->rd_istemp)
-        MyXactAccessedTempRel = true;
+        enlistRelationIdFor2PCChecks(relationId);

     pgstat_initstats(r);

Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257.2.2
diff -u -r1.257.2.2 xact.c
--- src/backend/access/transam/xact.c    26 Apr 2008 23:35:33 -0000    1.257.2.2
+++ src/backend/access/transam/xact.c    6 Oct 2008 20:39:46 -0000
@@ -62,13 +62,6 @@
 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
@@ -206,6 +199,10 @@
  */
 static MemoryContext TransactionAbortContext = NULL;

+#define    MAX_TEMP_TABLES_IN_A_TX    10
+#define    UNUSED_OID                0
+Oid        accessedTempRel[MAX_TEMP_TABLES_IN_A_TX]; /* Oids of accessed temporary relations */
+
 /*
  * List of add-on start- and end-of-xact callbacks
  */
@@ -1512,7 +1509,11 @@
     XactIsoLevel = DefaultXactIsoLevel;
     XactReadOnly = DefaultXactReadOnly;
     forceSyncCommit = false;
-    MyXactAccessedTempRel = false;
+    {
+        int i;
+        for (i = 0 ; i < MAX_TEMP_TABLES_IN_A_TX ; i++)
+            accessedTempRel[i] = UNUSED_OID;
+    }

     /*
      * reinitialize within-transaction counters
@@ -1777,6 +1778,38 @@
     RESUME_INTERRUPTS();
 }

+/* ----------------
+ *     enlistRelationIdFor2PCChecks - enlist a relation in the list of
+ *      resources to check at PREPARE COMMIT time if we are part of
+ *      a 2PC transaction. The resource will be removed from the list
+ *      if the table is dropped before commit.
+ * ----------------
+ */
+void
+enlistRelationIdFor2PCChecks(Oid relationId)
+{
+    /*
+     * Each time a temporary relation is accessed, it is added to the
+     * accessedTempRel list. PREPARE TRANSACTION will fail if any
+     * of the accessed relation is still valid (not dropped).  (This is
+     * called from from heapam.c.)
+     */
+    int  i;
+    bool fullOidList = true;
+
+    for (i = 0 ; i < MAX_TEMP_TABLES_IN_A_TX ; i++)
+    { /* Enlist the relation id in the first available slot */
+        if ((accessedTempRel[i] == UNUSED_OID) ||
+            (accessedTempRel[i] == relationId))
+        {
+            accessedTempRel[i] = relationId;
+            fullOidList = false;
+            break;
+        }
+    }
+    if (fullOidList)
+        elog(ERROR, "Too many temp tables accessed inside the same transaction. Increase MAX_TEMP_TABLES_IN_A_TX in
xact.c,list is full"); 
+}

 /*
  *    PrepareTransaction
@@ -1853,14 +1886,36 @@
      * 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.
+     * We only allow to proceed further if the accessed temp tables have
+     * been dropped before PREPARE COMMIT.
      */
-    if (MyXactAccessedTempRel)
-        ereport(ERROR,
-                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                 errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+    {
+        int  i;
+        for (i = 0; i < MAX_TEMP_TABLES_IN_A_TX; i++)
+        { /* Check all accessed temp tables */
+            if (accessedTempRel[i] != UNUSED_OID)
+            { /* If the table has been dropped, try_relation_open will fail and
+                 we can safely continue. */
+                Relation tempTable = try_relation_open(accessedTempRel[i], NoLock);
+                if (tempTable != NULL)
+                { /* We have an open temp table at PREPARE COMMIT time. We
+                     will only accept empty temp tables and throw an error
+                     in other cases. */
+                    HeapScanDesc scan;
+                    HeapTuple tuple;
+                    scan = heap_beginscan(tempTable, SnapshotNow, 0, NULL);
+                    if ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+                    {
+                        ereport(ERROR,
+                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("cannot PREPARE a transaction that has operated on temporary tables
thatare not empty at commit time"))); 
+                    }
+                    heap_endscan(scan);
+                    relation_close(tempTable, NoLock);
+                }
+            }
+        }
+    }

     /* Prevent cancel/die interrupt while cleaning up */
     HOLD_INTERRUPTS();
\echo Creation of a persistent table (not temp)
begin;
create table paul(x int);
insert into paul values(1);
prepare transaction 'persistentTableShouldSucceed';
commit prepared 'persistentTableShouldSucceed';

\echo Drop of a persistent table (not temp)
begin;
drop table paul;
prepare transaction 'dropPersistentTableShouldSucceed';
commit prepared 'dropPersistentTableShouldSucceed';

\echo Transaction-scope dropped temp table use case
begin;
create temp table foo(x int);
insert into foo values(1);
drop table foo;
prepare transaction 'dropTempTableShouldSucceed';
commit prepared 'dropTempTableShouldSucceed';

\echo Session-scope temp table use case
create temp table foo(x int);
begin;
insert into foo values(1);
delete from foo;
prepare transaction 'dropTempTableShouldSucceed';
commit prepared 'dropTempTableShouldSucceed';
drop table foo;

\echo Temp table with ON COMMIT DROP option
begin;
create temp table foo(x int) on commit drop;
insert into foo values(1);
prepare transaction 'onCommitDropTempTableShouldSucceed';
commit prepared 'onCommitDropTempTableShouldSucceed';

\echo Temp table with ON DELETE ROWS option (transaction scope)
begin;
create temp table foo(x int) on commit delete rows;
insert into foo values(1);
prepare transaction 'onCommitDeleteRowsTempTableShouldSucceed';
commit prepared 'onCommitDeleteRowsTempTableShouldSucceed';
drop table foo;

\echo Temp table with ON DELETE ROWS option (session scope)
create temp table foo(x int) on commit delete rows;
begin;
insert into foo values(1);
prepare transaction 'onCommitDeleteRowsTempTableShouldSucceed';
commit prepared 'onCommitDeleteRowsTempTableShouldSucceed';
drop table foo;

\echo Rollback to savepoint test case
BEGIN;
SAVEPOINT sp;
CREATE TEMP TABLE foo(bar int4);
ROLLBACK TO sp;
PREPARE TRANSACTION 'savepointTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php';
COMMIT PREPARED 'savepointTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php';

\echo Dirty buffer check
begin;
create temp table foo(a int, b int, c int) on commit drop;
select relname, relfilenode from pg_class where relname='foo';
insert into foo values(1,1,1);
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
insert into foo select * from foo;
prepare transaction 'bcd';
commit prepared 'bcd';
begin;
create temp table bar(a int, b int, c int) on commit drop;
select relname, relfilenode from pg_class where relname='bar';
insert into bar values(1,1,1);
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
insert into bar select * from bar;
commit;

\echo Existing non-empty temp table at commit time should still fail
begin;
create temp table foo(x int);
insert into foo values(1);
prepare transaction 'existingTempTableShouldFail';
commit prepared 'existingTempTableShouldFail';