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 
are not 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_rollback 2pc_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';
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to