Re: Transactions and temp tables

Hi,

> On Tue, Oct 07, 2008 at 04:57:37PM -0400, Emmanuel Cecchet wrote:
>
>> Heikki,
>>
>> Here is a new version of the patch using a hash table as you
>> suggested.  I also include the tests that I have added to the
>> regression test suite  to test the various scenarios.  All patches
>> are based on Postgres 8.3.3, let me know if you want me to  generate
>> patch for 8.4.
>>
>
> CVS TIP is the only place where new features, like this, go :)
>
I looked at the Wiki and it looks like I should add en entry (assuming I
get a patch for the current CVS HEAD) to CommitFest 2008-11. Is that
correct?
> I didn't see the attachment anyhow...
>
Good point! The same with the attachments now!

Thanks,
manu
### 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    7 Oct 2008 20:29:47 -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    7 Oct 2008 20:29:47 -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    7 Oct 2008 20:29:47 -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,9 @@
  */
 static MemoryContext TransactionAbortContext = NULL;

+/* Hash table containing Oids of accessed temporary relations */
+HTAB    *accessedTempRel;
+
 /*
  * List of add-on start- and end-of-xact callbacks
  */
@@ -1512,7 +1508,6 @@
     XactIsoLevel = DefaultXactIsoLevel;
     XactReadOnly = DefaultXactReadOnly;
     forceSyncCommit = false;
-    MyXactAccessedTempRel = false;

     /*
      * reinitialize within-transaction counters
@@ -1777,6 +1772,35 @@
     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.)
+     */
+    if (accessedTempRel == NULL)
+    { // Allocate the list on-demand
+        HASHCTL ctl;
+
+        ctl.keysize = sizeof(Oid);
+        ctl.entrysize = sizeof(Oid);
+        accessedTempRel = hash_create("accessed temp tables", 4, &ctl,
+                HASH_ELEM);
+    }
+
+    // Add to the hash list if missing
+    hash_search(accessedTempRel, &relationId, HASH_ENTER, NULL);
+}

 /*
  *    PrepareTransaction
@@ -1853,14 +1877,45 @@
      * 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")));
+    if (accessedTempRel != NULL)
+    {
+        HASH_SEQ_STATUS    status;
+        Oid*            tempTableOid;
+
+        /* Prevent further updates to the list as recommended in hash_seq_init doc. */
+        hash_freeze(accessedTempRel);
+        hash_seq_init(&status, accessedTempRel);
+        while ((tempTableOid = (Oid *) hash_seq_search(&status)) != NULL)
+        { /* Check all accessed temp tables. If the table has been dropped,
+           * try_relation_open will fail and we can safely continue. */
+            Relation tempTable = try_relation_open(*tempTableOid, 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)
+                {
+                    hash_seq_term(&status);
+                    hash_destroy(accessedTempRel);
+                    accessedTempRel = NULL;
+                    ereport(ERROR,
+                            (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                    errmsg("cannot PREPARE a transaction that has operated on temporary tables that
arenot empty at commit time"))); 
+                }
+                heap_endscan(scan);
+                relation_close(tempTable, NoLock);
+            }
+        }
+        hash_seq_term(&status);
+        hash_destroy(accessedTempRel);
+        accessedTempRel = NULL;
+    }

     /* Prevent cancel/die interrupt while cleaning up */
     HOLD_INTERRUPTS();
@@ -2149,6 +2204,12 @@
         elog(FATAL, "CleanupTransaction: unexpected state %s",
              TransStateAsString(s->state));

+    if (accessedTempRel == NULL)
+    {
+        hash_destroy(accessedTempRel);
+        accessedTempRel = NULL;
+    }
+
     /*
      * do abort cleanup processing
      */
### Eclipse Workspace Patch 1.0
#P Postgres-8.3.3
Index: src/test/regress/parallel_schedule
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/parallel_schedule,v
retrieving revision 1.46
diff -u -r1.46 parallel_schedule
--- src/test/regress/parallel_schedule    24 Nov 2007 19:49:23 -0000    1.46
+++ src/test/regress/parallel_schedule    7 Oct 2008 20:31:23 -0000
@@ -90,3 +90,11 @@

 # run tablespace by itself
 test: tablespace
+
+# --------
+# 2PC related tests
+# --------
+test: 2pc_persistent_table 2pc_on_delete_rows_transaction 2pc_on_commit_drop 2pc_temp_table_transaction
2pc_temp_table_rollback2pc_temp_table_savepoint 2pc_temp_table_failure 
+test: 2pc_dirty_buffer_check
+test: 2pc_temp_table_session
+test: 2pc_on_delete_rows_session
\ No newline at end of file
Index: src/test/regress/expected/2pc_on_delete_rows_session.out
===================================================================
RCS file: src/test/regress/expected/2pc_on_delete_rows_session.out
diff -N src/test/regress/expected/2pc_on_delete_rows_session.out
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ src/test/regress/expected/2pc_on_delete_rows_session.out    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,7 @@
+-- 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;
Index: src/test/regress/sql/2pc_persistent_table.sql
===================================================================
RCS file: src/test/regress/sql/2pc_persistent_table.sql
diff -N src/test/regress/sql/2pc_persistent_table.sql
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ src/test/regress/sql/2pc_persistent_table.sql    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,12 @@
+-- Creation of a persistent table (not temp)
+begin;
+create table paul(x int);
+insert into paul values(1);
+prepare transaction 'persistentTableShouldSucceed';
+commit prepared 'persistentTableShouldSucceed';
+
+-- Drop of a persistent table (not temp)
+begin;
+drop table paul;
+prepare transaction 'dropPersistentTableShouldSucceed';
+commit prepared 'dropPersistentTableShouldSucceed';
Index: src/test/regress/expected/2pc_temp_table_rollback.out
===================================================================
RCS file: src/test/regress/expected/2pc_temp_table_rollback.out
diff -N src/test/regress/expected/2pc_temp_table_rollback.out
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ src/test/regress/expected/2pc_temp_table_rollback.out    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,5 @@
+-- Rollback prepared
+BEGIN;
+CREATE TEMP TABLE foo(bar int4);
+PREPARE TRANSACTION 'rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php';
+ROLLBACK PREPARED 'rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php';
Index: src/test/regress/expected/2pc_temp_table_savepoint.out
===================================================================
RCS file: src/test/regress/expected/2pc_temp_table_savepoint.out
diff -N src/test/regress/expected/2pc_temp_table_savepoint.out
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ src/test/regress/expected/2pc_temp_table_savepoint.out    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,8 @@
+-- Rollback to savepoint test case
+BEGIN;
+SAVEPOINT sp;
+CREATE TEMP TABLE foo(bar int4);
+INSERT INTO foo VALUES (1);
+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';
Index: src/test/regress/expected/2pc_temp_table_failure.out
===================================================================
RCS file: src/test/regress/expected/2pc_temp_table_failure.out
diff -N src/test/regress/expected/2pc_temp_table_failure.out
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ src/test/regress/expected/2pc_temp_table_failure.out    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,8 @@
+-- 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';
+ERROR:  cannot PREPARE a transaction that has operated on temporary tables that are not empty at commit time
+commit prepared 'existingTempTableShouldFail';
+ERROR:  prepared transaction with identifier "existingTempTableShouldFail" does not exist
Index: src/test/regress/sql/2pc_on_commit_drop.sql
===================================================================
RCS file: src/test/regress/sql/2pc_on_commit_drop.sql
diff -N src/test/regress/sql/2pc_on_commit_drop.sql
--- /dev/null    1 Jan 1970 00:00:00 -0000
+++ src/test/regress/sql/2pc_on_commit_drop.sql    1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,6 @@
+-- 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';