Thread: Transactions and temp tables
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';
Emmanuel Cecchet wrote: > 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. Thanks for looking into this. The patch allows preparing any transaction that has dropped the temp table, even if it wasn't created in the same transaction. Is that sane? Also, even if the table is created and dropped in the same transaction, a subsequent transaction that tries to create and drop the table gets blocked on the lock. I suppose we could just say that that's the way it works, but I'm afraid it will come as a nasty surprise, making the feature a lot less useful. The fixed-size array of temp table oids is an unnecessary limitation. A list or hash table would be better. > Let me know what you think of the patch and if it > could be applied to 8.3 and 8.4? Not to 8.3. We only back-patch bug-fixes, and this isn't one. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Hi Heikki, > The patch allows preparing any transaction that has dropped the temp > table, even if it wasn't created in the same transaction. Is that sane? If you have a temp table created with an 'on commit delete rows' option in another transaction, it would be fine to drop it in another transaction. If the temp table was created without any on commit option, it could only cross prepare commit if it is empty and then it could be safely dropped in another transaction. That does not seem to insane for me if you need temp tables for a session. > Also, even if the table is created and dropped in the same > transaction, a subsequent transaction that tries to create and drop > the table gets blocked on the lock. I suppose we could just say that > that's the way it works, but I'm afraid it will come as a nasty > surprise, making the feature a lot less useful. I do not get that one, if the table is dropped in the transaction the lock is released. Why would another transaction be blocked when trying to create/drop another temp table? When I run my test cases (see attached file in previous mail), I create/drop multiple times the same temp table in different transactions and I do not experience any blocking. > The fixed-size array of temp table oids is an unnecessary limitation. > A list or hash table would be better. You are right, I will fix that. >> Let me know what you think of the patch and if it could be applied to >> 8.3 and 8.4? > Not to 8.3. We only back-patch bug-fixes, and this isn't one. Ok understood. Thanks for your feedback and don't hesitate to enlighten me on the potential locking issue I did not understand. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
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. Thanks in advance for your feedback, Emmanuel Emmanuel Cecchet wrote: > Hi Heikki, > >> The patch allows preparing any transaction that has dropped the temp >> table, even if it wasn't created in the same transaction. Is that sane? > If you have a temp table created with an 'on commit delete rows' > option in another transaction, it would be fine to drop it in another > transaction. If the temp table was created without any on commit > option, it could only cross prepare commit if it is empty and then it > could be safely dropped in another transaction. That does not seem to > insane for me if you need temp tables for a session. >> Also, even if the table is created and dropped in the same >> transaction, a subsequent transaction that tries to create and drop >> the table gets blocked on the lock. I suppose we could just say that >> that's the way it works, but I'm afraid it will come as a nasty >> surprise, making the feature a lot less useful. > I do not get that one, if the table is dropped in the transaction the > lock is released. Why would another transaction be blocked when trying > to create/drop another temp table? > When I run my test cases (see attached file in previous mail), I > create/drop multiple times the same temp table in different > transactions and I do not experience any blocking. >> The fixed-size array of temp table oids is an unnecessary limitation. >> A list or hash table would be better. > You are right, I will fix that. >>> Let me know what you think of the patch and if it could be applied >>> to 8.3 and 8.4? >> Not to 8.3. We only back-patch bug-fixes, and this isn't one. > Ok understood. > > Thanks for your feedback and don't hesitate to enlighten me on the > potential locking issue I did not understand. > Emmanuel > -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
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 didn't see the attachment anyhow... Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
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';
On Tue, Oct 07, 2008 at 06:12:14PM -0400, Emmanuel Cecchet wrote: > 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! Perhaps we did not make this clear. The patch is a new feature. New features are not going into 8.3, as it has already been released. Make a patch against CVS TIP aka 8.4, and re-submit. Cheers, David -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Hi, Here are the patches for 8.4 (1 patch for the code and 1 patch for the regression tests). Thanks in advance for your feedback, Emmanuel David Fetter wrote: > On Tue, Oct 07, 2008 at 06:12:14PM -0400, Emmanuel Cecchet wrote: > >> 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! >> > > Perhaps we did not make this clear. The patch is a new feature. New > features are not going into 8.3, as it has already been released. > > Make a patch against CVS TIP aka 8.4, and re-submit. > > Cheers, > David > -- 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-HEAD 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 23:41:14 -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/serial_schedule =================================================================== RCS file: /root/cvsrepo/pgsql/src/test/regress/serial_schedule,v retrieving revision 1.43 diff -u -r1.43 serial_schedule --- src/test/regress/serial_schedule 24 Nov 2007 20:41:35 -0000 1.43 +++ src/test/regress/serial_schedule 7 Oct 2008 23:41:14 -0000 @@ -115,3 +115,13 @@ test: xml test: stats test: tablespace +test: 2pc_persistent_table +test: 2pc_on_delete_rows_transaction +test: 2pc_on_commit_drop +test: 2pc_temp_table_transaction +test: 2pc_temp_table_rollback +test: 2pc_temp_table_savepoint +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session +test: 2pc_temp_table_failure \ No newline at end of file Index: src/test/regress/sql/2pc_on_delete_rows_transaction.sql =================================================================== RCS file: src/test/regress/sql/2pc_on_delete_rows_transaction.sql diff -N src/test/regress/sql/2pc_on_delete_rows_transaction.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_on_delete_rows_transaction.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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; Index: src/test/regress/expected/2pc_on_commit_drop.out =================================================================== RCS file: src/test/regress/expected/2pc_on_commit_drop.out diff -N src/test/regress/expected/2pc_on_commit_drop.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_on_commit_drop.out 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'; Index: src/test/regress/expected/2pc_temp_table_transaction.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_transaction.out diff -N src/test/regress/expected/2pc_temp_table_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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'; Index: src/test/regress/sql/2pc_temp_table_failure.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_failure.sql diff -N src/test/regress/sql/2pc_temp_table_failure.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_failure.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,6 @@ +-- 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'; 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/sql/2pc_temp_table_transaction.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_transaction.sql diff -N src/test/regress/sql/2pc_temp_table_transaction.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_transaction.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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'; 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_temp_table_session.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_session.sql diff -N src/test/regress/sql/2pc_temp_table_session.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_session.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,8 @@ +-- 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; Index: src/test/regress/expected/2pc_dirty_buffer_check.out =================================================================== RCS file: src/test/regress/expected/2pc_dirty_buffer_check.out diff -N src/test/regress/expected/2pc_dirty_buffer_check.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_dirty_buffer_check.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,52 @@ +-- 2PC Dirty buffer check +begin; +create temp table foo(a int, b int, c int) on commit drop; +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; +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; 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'; Index: src/test/regress/sql/2pc_temp_table_savepoint.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_savepoint.sql diff -N src/test/regress/sql/2pc_temp_table_savepoint.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_savepoint.sql 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/sql/2pc_temp_table_rollback.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_rollback.sql diff -N src/test/regress/sql/2pc_temp_table_rollback.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_rollback.sql 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_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/sql/2pc_dirty_buffer_check.sql =================================================================== RCS file: src/test/regress/sql/2pc_dirty_buffer_check.sql diff -N src/test/regress/sql/2pc_dirty_buffer_check.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_dirty_buffer_check.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,52 @@ +-- 2PC Dirty buffer check +begin; +create temp table foo(a int, b int, c int) on commit drop; +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; +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; Index: src/test/regress/expected/2pc_persistent_table.out =================================================================== RCS file: src/test/regress/expected/2pc_persistent_table.out diff -N src/test/regress/expected/2pc_persistent_table.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_persistent_table.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,11 @@ +-- 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_session.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_session.out diff -N src/test/regress/expected/2pc_temp_table_session.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_session.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,8 @@ +-- 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; Index: src/test/regress/sql/2pc_on_delete_rows_session.sql =================================================================== RCS file: src/test/regress/sql/2pc_on_delete_rows_session.sql diff -N src/test/regress/sql/2pc_on_delete_rows_session.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_on_delete_rows_session.sql 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/expected/2pc_on_delete_rows_transaction.out =================================================================== RCS file: src/test/regress/expected/2pc_on_delete_rows_transaction.out diff -N src/test/regress/expected/2pc_on_delete_rows_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_on_delete_rows_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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; ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/backend/access/heap/heapam.c =================================================================== RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.264 diff -u -r1.264 heapam.c --- src/backend/access/heap/heapam.c 30 Sep 2008 10:52:10 -0000 1.264 +++ src/backend/access/heap/heapam.c 8 Oct 2008 01:53:44 -0000 @@ -878,7 +878,7 @@ /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -926,7 +926,7 @@ /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -976,7 +976,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.265 diff -u -r1.265 xact.c --- src/backend/access/transam/xact.c 11 Aug 2008 11:05:10 -0000 1.265 +++ src/backend/access/transam/xact.c 8 Oct 2008 01:53:44 -0000 @@ -47,6 +47,7 @@ #include "utils/memutils.h" #include "utils/relcache.h" #include "utils/snapmgr.h" +#include "utils/tqual.h" #include "utils/xml.h" #include "pg_trace.h" @@ -65,13 +66,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 @@ -209,6 +203,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 */ @@ -1511,7 +1508,6 @@ XactIsoLevel = DefaultXactIsoLevel; XactReadOnly = DefaultXactReadOnly; forceSyncCommit = false; - MyXactAccessedTempRel = false; /* * reinitialize within-transaction counters @@ -1777,6 +1773,36 @@ 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 + Oid *tid = hash_search(accessedTempRel, &relationId, HASH_ENTER, NULL); + *tid = relationId; +} /* * PrepareTransaction @@ -1853,14 +1879,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(); @@ -2151,6 +2208,12 @@ elog(FATAL, "CleanupTransaction: unexpected state %s", TransStateAsString(s->state)); + if (accessedTempRel != NULL) + { + hash_destroy(accessedTempRel); + accessedTempRel = NULL; + } + /* * do abort cleanup processing */ Index: src/include/access/xact.h =================================================================== RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v retrieving revision 1.95 diff -u -r1.95 xact.h --- src/include/access/xact.h 11 Aug 2008 11:05:11 -0000 1.95 +++ src/include/access/xact.h 8 Oct 2008 01:53:44 -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
Emmanuel Cecchet wrote: >> Also, even if the table is created and dropped in the same >> transaction, a subsequent transaction that tries to create and drop >> the table gets blocked on the lock. I suppose we could just say that >> that's the way it works, but I'm afraid it will come as a nasty >> surprise, making the feature a lot less useful. > I do not get that one, if the table is dropped in the transaction the > lock is released. Why would another transaction be blocked when trying > to create/drop another temp table? I was thinking of a transaction that's just prepared (1st phase), but not committed or rolled back: postgres=# CREATE TEMP TABLE foo (bar int); CREATE TABLE postgres=# BEGIN; BEGIN postgres=# DROP TABLE foo; DROP TABLE postgres=# PREPARE TRANSACTION '2pc'; PREPARE TRANSACTION postgres=# SELECT * FROM foo; <blocks> Furthermore, it looks like the backend refuses to shut down, even if you end the psql session, because RemoveTempRelations() is called on backend shutdown and it gets blocked on that lock. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: > I was thinking of a transaction that's just prepared (1st phase), but > not committed or rolled back: > > postgres=# CREATE TEMP TABLE foo (bar int); > CREATE TABLE > postgres=# BEGIN; > BEGIN > postgres=# DROP TABLE foo; > DROP TABLE > postgres=# PREPARE TRANSACTION '2pc'; > PREPARE TRANSACTION > postgres=# SELECT * FROM foo; > <blocks> > > Furthermore, it looks like the backend refuses to shut down, even if > you end the psql session, because RemoveTempRelations() is called on > backend shutdown and it gets blocked on that lock. Thanks for the example, I get it now. Does it make sense to allow any request execution between PREPARE TRANSACTION and the subsequent COMMIT or ROLLBACK? I did the same experiment with a regular table (not a temp table) and it blocks exactly the same way, so I don't think that the problem is specific to temp tables. Once PREPARE has been executed, the transaction state is restored to TRANS_DEFAULT, but I wonder if we should not have a specific TRANS_PREPARED state in which we can only authorize a COMMIT or a ROLLBACK. Is there any reasonable use case where someone would have to execute requests between PREPARE and COMMIT/ROLLBACK? Let me know what you think of the additional TRANS_PREPARED transaction state. It looks like the behavior of what happens between PREPARE and COMMIT/ROLLBACK is pretty much undefined. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Emmanuel Cecchet <manu@frogthinker.org> writes:
> Thanks for the example, I get it now. Does it make sense to allow any
> request execution between PREPARE TRANSACTION and the subsequent COMMIT
> or ROLLBACK?
Yes. Don't even think of trying to disallow that. The COMMIT doesn't
even have to happen in the same session, anyway.
regards, tom lane
Tom Lane wrote: > Emmanuel Cecchet <manu@frogthinker.org> writes: > >> Thanks for the example, I get it now. Does it make sense to allow any >> request execution between PREPARE TRANSACTION and the subsequent COMMIT >> or ROLLBACK? >> > > Yes. Don't even think of trying to disallow that. The COMMIT doesn't > even have to happen in the same session, anyway. > Ok, so actually I don't see any different behavior between a temp table or a regular table. The locking happens the same way and as long as the commit prepared happens (potentially in another session), the lock is released at commit time which seems to make sense. The issue that Heikki was mentioning about the server not shutting down seems to happen as soon as you have a single transaction that has prepared commit but not commit/rollback yet. This seems also independent of whether you are using a temp table or not. It seems that the patch did not alter the behavior of PG in that regard. What do you think? Emmanuel
Emmanuel Cecchet <manu@frogthinker.org> writes:
> Ok, so actually I don't see any different behavior between a temp table
> or a regular table. The locking happens the same way and as long as the
> commit prepared happens (potentially in another session), the lock is
> released at commit time which seems to make sense.
Right, the problem is that you can't shut down the original backend
because it'll try to drop the temp table at exit, and then block on
the lock that the prepared xact is holding. From a database management
standpoint that is unacceptable --- it means for instance that you can't
shut down the database cleanly while such a prepared transaction is
pending. The difference from a regular table is that no such automatic
action is taken at backend exit for regular tables.
The whole business of having restrictions on temp table access is
annoying; I wish we could get rid of them not add complexity to
enforcing them. The local-buffer-management end of the issue seems
readily solvable: we need only decree that PREPARE has to flush out any
dirty local buffers (and maybe discard local buffers altogether, not
sure). But I've not been able to see a decent solution to the lock
behavior.
regards, tom lane
Hi,
I am attaching a new patch that deals with the issue of the locks on
temporary tables that have been accessed in transactions that have been
prepared but not committed.
I have added another list that keeps track of temp tables accessed by
transactions that are prepared but not committed. The RemoveTempTable
callback does not try to acquire locks on these tables. Now postmaster
can terminate even with transactions in the prepared state that accessed
temp tables.
Let me know what you think.
manu
Tom Lane wrote:
> Emmanuel Cecchet <manu@frogthinker.org> writes:
>
>> Ok, so actually I don't see any different behavior between a temp table
>> or a regular table. The locking happens the same way and as long as the
>> commit prepared happens (potentially in another session), the lock is
>> released at commit time which seems to make sense.
>>
>
> Right, the problem is that you can't shut down the original backend
> because it'll try to drop the temp table at exit, and then block on
> the lock that the prepared xact is holding. From a database management
> standpoint that is unacceptable --- it means for instance that you can't
> shut down the database cleanly while such a prepared transaction is
> pending. The difference from a regular table is that no such automatic
> action is taken at backend exit for regular tables.
>
> The whole business of having restrictions on temp table access is
> annoying; I wish we could get rid of them not add complexity to
> enforcing them. The local-buffer-management end of the issue seems
> readily solvable: we need only decree that PREPARE has to flush out any
> dirty local buffers (and maybe discard local buffers altogether, not
> sure). But I've not been able to see a decent solution to the lock
> behavior.
>
> regards, tom lane
>
>
### Eclipse Workspace Patch 1.0
#P Postgres-HEAD
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.264
diff -u -r1.264 heapam.c
--- src/backend/access/heap/heapam.c 30 Sep 2008 10:52:10 -0000 1.264
+++ src/backend/access/heap/heapam.c 9 Oct 2008 21:44:04 -0000
@@ -878,7 +878,7 @@
/* Make note that we've accessed a temporary relation */
if (r->rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
@@ -926,7 +926,7 @@
/* Make note that we've accessed a temporary relation */
if (r->rd_istemp)
- MyXactAccessedTempRel = true;
+ enlistRelationIdFor2PCChecks(relationId);
pgstat_initstats(r);
@@ -976,7 +976,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.265
diff -u -r1.265 xact.c
--- src/backend/access/transam/xact.c 11 Aug 2008 11:05:10 -0000 1.265
+++ src/backend/access/transam/xact.c 9 Oct 2008 21:44:04 -0000
@@ -47,6 +47,7 @@
#include "utils/memutils.h"
#include "utils/relcache.h"
#include "utils/snapmgr.h"
+#include "utils/tqual.h"
#include "utils/xml.h"
#include "pg_trace.h"
@@ -65,13 +66,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
@@ -209,6 +203,13 @@
*/
static MemoryContext TransactionAbortContext = NULL;
+/* Hash table containing Oids of accessed temporary relations */
+HTAB *accessedTempRel;
+/* Hash table containing Oids of accessed temporary relations that have been
+ * prepared commit but not committed yet
+ */
+HTAB *preparedTempRel;
+
/*
* List of add-on start- and end-of-xact callbacks
*/
@@ -250,6 +251,7 @@
SubTransactionId mySubid,
SubTransactionId parentSubid);
static void CleanupTransaction(void);
+static void CleanupAccessedTempRel(void);
static void CommitTransaction(void);
static TransactionId RecordTransactionAbort(bool isSubXact);
static void StartTransaction(void);
@@ -624,7 +626,7 @@
/* Propagate new command ID into static snapshots */
SnapshotSetCommandId(currentCommandId);
-
+
/*
* Make any catalog changes done by the just-completed command
* visible in the local syscache. We obviously don't need to do
@@ -1119,10 +1121,10 @@
*/
if (s->parent->childXids == NULL)
new_childXids =
- MemoryContextAlloc(TopTransactionContext,
+ MemoryContextAlloc(TopTransactionContext,
new_maxChildXids * sizeof(TransactionId));
else
- new_childXids = repalloc(s->parent->childXids,
+ new_childXids = repalloc(s->parent->childXids,
new_maxChildXids * sizeof(TransactionId));
s->parent->childXids = new_childXids;
@@ -1511,7 +1513,6 @@
XactIsoLevel = DefaultXactIsoLevel;
XactReadOnly = DefaultXactReadOnly;
forceSyncCommit = false;
- MyXactAccessedTempRel = false;
/*
* reinitialize within-transaction counters
@@ -1760,6 +1761,8 @@
AtCommit_Memory();
+ CleanupAccessedTempRel();
+
s->transactionId = InvalidTransactionId;
s->subTransactionId = InvalidSubTransactionId;
s->nestingLevel = 0;
@@ -1777,6 +1780,74 @@
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)
+{
+ Oid *tid;
+
+ /*
+ * 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
+ tid = hash_search(accessedTempRel, &relationId, HASH_ENTER, NULL);
+ *tid = relationId;
+}
+
+/*
+ * Cleanup the list of prepared temp tables that were accessed during this transaction.
+ */
+static void CleanupAccessedTempRel(void)
+{
+ if (accessedTempRel != NULL)
+ {
+ if (preparedTempRel != NULL)
+ { // We have prepared transactions with temp tables
+ HASH_SEQ_STATUS status;
+ Oid* tempTableOid;
+
+ hash_seq_init(&status, accessedTempRel);
+ while ((tempTableOid = (Oid *) hash_seq_search(&status)) != NULL)
+ { // Add all relations to the hash list
+ hash_search(preparedTempRel, tempTableOid, HASH_REMOVE, NULL);
+ }
+ }
+ hash_destroy(accessedTempRel);
+ accessedTempRel = NULL;
+ }
+}
+
+/*
+ * Returns true if the provided relationId is a temp table that has been
+ * prepared commit but not committed yet.
+ */
+bool
+isPreparedTempRelation(Oid relationId)
+{
+ if (preparedTempRel == NULL)
+ return false;
+
+ return hash_search(preparedTempRel, &relationId, HASH_FIND, NULL) != NULL;
+}
/*
* PrepareTransaction
@@ -1853,14 +1924,61 @@
* 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);
+ }
+ }
+
+ // Success! All temp relations are empty, we need to add them to the list of prepared commit temp relations
+ if (preparedTempRel == NULL)
+ { // Create list if it does not exist yet
+ HASHCTL ctl;
+
+ ctl.keysize = sizeof(Oid);
+ ctl.entrysize = sizeof(Oid);
+ preparedTempRel = hash_create("prepared temp tables", 10, &ctl,
+ HASH_ELEM);
+ }
+
+ hash_seq_init(&status, accessedTempRel);
+ while ((tempTableOid = (Oid *) hash_seq_search(&status)) != NULL)
+ { // Add all relations to the hash list
+ Oid *tid = hash_search(preparedTempRel, tempTableOid, HASH_ENTER, NULL);
+ *tid = *tempTableOid;
+ }
+ }
/* Prevent cancel/die interrupt while cleaning up */
HOLD_INTERRUPTS();
@@ -2151,6 +2269,14 @@
elog(FATAL, "CleanupTransaction: unexpected state %s",
TransStateAsString(s->state));
+ if (accessedTempRel != NULL)
+ {
+ CleanupAccessedTempRel();
+
+ hash_destroy(accessedTempRel);
+ accessedTempRel = NULL;
+ }
+
/*
* do abort cleanup processing
*/
Index: src/include/access/xact.h
===================================================================
RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v
retrieving revision 1.95
diff -u -r1.95 xact.h
--- src/include/access/xact.h 11 Aug 2008 11:05:11 -0000 1.95
+++ src/include/access/xact.h 9 Oct 2008 21:44:04 -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,10 @@
/* 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);
+/* Check if a temp table has been prepared for 2PC but not committed yet */
+extern bool isPreparedTempRelation(Oid relationId);
/*
* start- and end-of-transaction callbacks for dynamically loaded modules
Index: src/backend/catalog/dependency.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/backend/catalog/dependency.c,v
retrieving revision 1.81
diff -u -r1.81 dependency.c
--- src/backend/catalog/dependency.c 4 Oct 2008 21:56:52 -0000 1.81
+++ src/backend/catalog/dependency.c 9 Oct 2008 21:44:04 -0000
@@ -677,6 +677,12 @@
otherObject.objectId = foundDep->objid;
otherObject.objectSubId = foundDep->objsubid;
+ if (isPreparedTempRelation(foundDep->objid))
+ {
+ elog(DEBUG3, "temp relation %d is already prepared, ignoring", foundDep->objid);
+ continue;
+ }
+
/*
* Must lock the dependent object before recursing to it.
*/
Hi all, Here is the latest patch and the regression tests for the temp tables and 2PC issue. Is there a way to stop and restart postmaster between 2 tests? Thanks for your feedback, Emmanuel -- 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-HEAD Index: src/test/regress/parallel_schedule =================================================================== RCS file: /root/cvsrepo/pgsql/src/test/regress/parallel_schedule,v retrieving revision 1.49 diff -u -r1.49 parallel_schedule --- src/test/regress/parallel_schedule 4 Oct 2008 21:56:55 -0000 1.49 +++ src/test/regress/parallel_schedule 10 Oct 2008 18:33:01 -0000 @@ -90,3 +90,19 @@ # 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 +# The following tests must be executing in sequence, +# do not alter the order nor try to execute them in parallel +test: 2pc_temp_table_prepared_not_committed +test: 2pc_temp_table_commit_prepared +# This test must be run last to check if the database properly +# shutdowns with a prepared transaction that is not committed +test: 2pc_temp_table_prepared_not_committed + \ No newline at end of file Index: src/test/regress/serial_schedule =================================================================== RCS file: /root/cvsrepo/pgsql/src/test/regress/serial_schedule,v retrieving revision 1.46 diff -u -r1.46 serial_schedule --- src/test/regress/serial_schedule 4 Oct 2008 21:56:55 -0000 1.46 +++ src/test/regress/serial_schedule 10 Oct 2008 18:33:01 -0000 @@ -118,3 +118,20 @@ test: xml test: stats test: tablespace +test: 2pc_persistent_table +test: 2pc_on_delete_rows_transaction +test: 2pc_on_commit_drop +test: 2pc_temp_table_transaction +test: 2pc_temp_table_rollback +test: 2pc_temp_table_savepoint +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session +test: 2pc_temp_table_failure +# The following tests must be executing in sequence, +# do not alter the order nor try to execute them in parallel +test: 2pc_temp_table_prepared_not_committed +test: 2pc_temp_table_commit_prepared +# This test must be run last to check if the database properly +# shutdowns with a prepared transaction that is not committed +test: 2pc_temp_table_prepared_not_committed 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/expected/2pc_temp_table_commit_prepared.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_commit_prepared.out diff -N src/test/regress/expected/2pc_temp_table_commit_prepared.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_commit_prepared.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,8 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- This test has to be called in sequence after 2pc_temp_table_prepared_not_committed.sql +commit prepared 'preparedNonCommittedFoo'; +-- The table should not exist anymore in this session +drop table foo; +ERROR: table "foo" does not exist 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/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/sql/2pc_dirty_buffer_check.sql =================================================================== RCS file: src/test/regress/sql/2pc_dirty_buffer_check.sql diff -N src/test/regress/sql/2pc_dirty_buffer_check.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_dirty_buffer_check.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,52 @@ +-- 2PC Dirty buffer check +begin; +create temp table foo(a int, b int, c int) on commit drop; +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; +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; Index: src/test/regress/expected/2pc_temp_table_transaction.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_transaction.out diff -N src/test/regress/expected/2pc_temp_table_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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'; Index: src/test/regress/sql/2pc_temp_table_rollback.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_rollback.sql diff -N src/test/regress/sql/2pc_temp_table_rollback.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_rollback.sql 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/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'; Index: src/test/regress/expected/2pc_temp_table_prepared_not_committed.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_prepared_not_committed.out diff -N src/test/regress/expected/2pc_temp_table_prepared_not_committed.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_prepared_not_committed.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,10 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- Session-scope temp table use case but exit after prepared +-- see 2pc_temp_table_commit_prepared.sql for committing that transaction +create temp table foo(x int); +begin; +insert into foo values(1); +delete from foo; +prepare transaction 'preparedNonCommittedFoo'; Index: src/test/regress/sql/2pc_on_delete_rows_transaction.sql =================================================================== RCS file: src/test/regress/sql/2pc_on_delete_rows_transaction.sql diff -N src/test/regress/sql/2pc_on_delete_rows_transaction.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_on_delete_rows_transaction.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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; Index: src/test/regress/expected/2pc_dirty_buffer_check.out =================================================================== RCS file: src/test/regress/expected/2pc_dirty_buffer_check.out diff -N src/test/regress/expected/2pc_dirty_buffer_check.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_dirty_buffer_check.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,52 @@ +-- 2PC Dirty buffer check +begin; +create temp table foo(a int, b int, c int) on commit drop; +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; +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; Index: src/test/regress/sql/2pc_on_delete_rows_session.sql =================================================================== RCS file: src/test/regress/sql/2pc_on_delete_rows_session.sql diff -N src/test/regress/sql/2pc_on_delete_rows_session.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_on_delete_rows_session.sql 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/expected/2pc_on_commit_drop.out =================================================================== RCS file: src/test/regress/expected/2pc_on_commit_drop.out diff -N src/test/regress/expected/2pc_on_commit_drop.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_on_commit_drop.out 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'; Index: src/test/regress/sql/2pc_temp_table_commit_prepared.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_commit_prepared.sql diff -N src/test/regress/sql/2pc_temp_table_commit_prepared.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_commit_prepared.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- This test has to be called in sequence after 2pc_temp_table_prepared_not_committed.sql +commit prepared 'preparedNonCommittedFoo'; +-- The table should not exist anymore in this session +drop table foo; Index: src/test/regress/expected/2pc_on_delete_rows_transaction.out =================================================================== RCS file: src/test/regress/expected/2pc_on_delete_rows_transaction.out diff -N src/test/regress/expected/2pc_on_delete_rows_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_on_delete_rows_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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; Index: src/test/regress/sql/2pc_temp_table_transaction.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_transaction.sql diff -N src/test/regress/sql/2pc_temp_table_transaction.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_transaction.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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'; Index: src/test/regress/sql/2pc_temp_table_prepared_not_committed.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_prepared_not_committed.sql diff -N src/test/regress/sql/2pc_temp_table_prepared_not_committed.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_prepared_not_committed.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,10 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- Session-scope temp table use case but exit after prepared +-- see 2pc_temp_table_commit_prepared.sql for committing that transaction +create temp table foo(x int); +begin; +insert into foo values(1); +delete from foo; +prepare transaction 'preparedNonCommittedFoo'; Index: src/test/regress/sql/2pc_temp_table_savepoint.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_savepoint.sql diff -N src/test/regress/sql/2pc_temp_table_savepoint.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_savepoint.sql 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/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_persistent_table.out =================================================================== RCS file: src/test/regress/expected/2pc_persistent_table.out diff -N src/test/regress/expected/2pc_persistent_table.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_persistent_table.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,11 @@ +-- 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/sql/2pc_temp_table_failure.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_failure.sql diff -N src/test/regress/sql/2pc_temp_table_failure.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_failure.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,6 @@ +-- 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'; 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_session.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_session.out diff -N src/test/regress/expected/2pc_temp_table_session.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_session.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,8 @@ +-- 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; Index: src/test/regress/sql/2pc_temp_table_session.sql =================================================================== RCS file: src/test/regress/sql/2pc_temp_table_session.sql diff -N src/test/regress/sql/2pc_temp_table_session.sql --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/sql/2pc_temp_table_session.sql 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,8 @@ +-- 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; ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/backend/access/heap/heapam.c =================================================================== RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.264 diff -u -r1.264 heapam.c --- src/backend/access/heap/heapam.c 30 Sep 2008 10:52:10 -0000 1.264 +++ src/backend/access/heap/heapam.c 9 Oct 2008 21:44:04 -0000 @@ -878,7 +878,7 @@ /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -926,7 +926,7 @@ /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -976,7 +976,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.265 diff -u -r1.265 xact.c --- src/backend/access/transam/xact.c 11 Aug 2008 11:05:10 -0000 1.265 +++ src/backend/access/transam/xact.c 9 Oct 2008 21:44:04 -0000 @@ -47,6 +47,7 @@ #include "utils/memutils.h" #include "utils/relcache.h" #include "utils/snapmgr.h" +#include "utils/tqual.h" #include "utils/xml.h" #include "pg_trace.h" @@ -65,13 +66,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 @@ -209,6 +203,13 @@ */ static MemoryContext TransactionAbortContext = NULL; +/* Hash table containing Oids of accessed temporary relations */ +HTAB *accessedTempRel; +/* Hash table containing Oids of accessed temporary relations that have been + * prepared commit but not committed yet + */ +HTAB *preparedTempRel; + /* * List of add-on start- and end-of-xact callbacks */ @@ -250,6 +251,7 @@ SubTransactionId mySubid, SubTransactionId parentSubid); static void CleanupTransaction(void); +static void CleanupAccessedTempRel(void); static void CommitTransaction(void); static TransactionId RecordTransactionAbort(bool isSubXact); static void StartTransaction(void); @@ -1511,7 +1513,6 @@ XactIsoLevel = DefaultXactIsoLevel; XactReadOnly = DefaultXactReadOnly; forceSyncCommit = false; - MyXactAccessedTempRel = false; /* * reinitialize within-transaction counters @@ -1760,6 +1761,8 @@ AtCommit_Memory(); + CleanupAccessedTempRel(); + s->transactionId = InvalidTransactionId; s->subTransactionId = InvalidSubTransactionId; s->nestingLevel = 0; @@ -1777,6 +1780,74 @@ 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) +{ + Oid *tid; + + /* + * 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 + tid = hash_search(accessedTempRel, &relationId, HASH_ENTER, NULL); + *tid = relationId; +} + +/* + * Cleanup the list of prepared temp tables that were accessed during this transaction. + */ +static void CleanupAccessedTempRel(void) +{ + if (accessedTempRel != NULL) + { + if (preparedTempRel != NULL) + { // We have prepared transactions with temp tables + HASH_SEQ_STATUS status; + Oid* tempTableOid; + + hash_seq_init(&status, accessedTempRel); + while ((tempTableOid = (Oid *) hash_seq_search(&status)) != NULL) + { // Add all relations to the hash list + hash_search(preparedTempRel, tempTableOid, HASH_REMOVE, NULL); + } + } + hash_destroy(accessedTempRel); + accessedTempRel = NULL; + } +} + +/* + * Returns true if the provided relationId is a temp table that has been + * prepared commit but not committed yet. + */ +bool +isPreparedTempRelation(Oid relationId) +{ + if (preparedTempRel == NULL) + return false; + + return hash_search(preparedTempRel, &relationId, HASH_FIND, NULL) != NULL; +} /* * PrepareTransaction @@ -1853,14 +1924,61 @@ * 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); + } + } + + // Success! All temp relations are empty, we need to add them to the list of prepared commit temp relations + if (preparedTempRel == NULL) + { // Create list if it does not exist yet + HASHCTL ctl; + + ctl.keysize = sizeof(Oid); + ctl.entrysize = sizeof(Oid); + preparedTempRel = hash_create("prepared temp tables", 10, &ctl, + HASH_ELEM); + } + + hash_seq_init(&status, accessedTempRel); + while ((tempTableOid = (Oid *) hash_seq_search(&status)) != NULL) + { // Add all relations to the hash list + Oid *tid = hash_search(preparedTempRel, tempTableOid, HASH_ENTER, NULL); + *tid = *tempTableOid; + } + } /* Prevent cancel/die interrupt while cleaning up */ HOLD_INTERRUPTS(); @@ -2151,6 +2269,14 @@ elog(FATAL, "CleanupTransaction: unexpected state %s", TransStateAsString(s->state)); + if (accessedTempRel != NULL) + { + CleanupAccessedTempRel(); + + hash_destroy(accessedTempRel); + accessedTempRel = NULL; + } + /* * do abort cleanup processing */ Index: src/include/access/xact.h =================================================================== RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v retrieving revision 1.95 diff -u -r1.95 xact.h --- src/include/access/xact.h 11 Aug 2008 11:05:11 -0000 1.95 +++ src/include/access/xact.h 9 Oct 2008 21:44:04 -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,10 @@ /* 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); +/* Check if a temp table has been prepared for 2PC but not committed yet */ +extern bool isPreparedTempRelation(Oid relationId); /* * start- and end-of-transaction callbacks for dynamically loaded modules Index: src/backend/catalog/dependency.c =================================================================== RCS file: /root/cvsrepo/pgsql/src/backend/catalog/dependency.c,v retrieving revision 1.81 diff -u -r1.81 dependency.c --- src/backend/catalog/dependency.c 4 Oct 2008 21:56:52 -0000 1.81 +++ src/backend/catalog/dependency.c 9 Oct 2008 21:44:04 -0000 @@ -677,6 +677,12 @@ otherObject.objectId = foundDep->objid; otherObject.objectSubId = foundDep->objsubid; + if (isPreparedTempRelation(foundDep->objid)) + { + elog(DEBUG3, "temp relation %d is already prepared, ignoring", foundDep->objid); + continue; + } + /* * Must lock the dependent object before recursing to it. */
Emmanuel Cecchet wrote: > Here is the latest patch and the regression tests for the temp tables > and 2PC issue. > Is there a way to stop and restart postmaster between 2 tests? > > Thanks for your feedback, > Emmanuel I did not get any comment on that one. How should I proceed so that the patch integration can be considered for 8.4? Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Emmanuel Cecchet wrote: > Emmanuel Cecchet wrote: >> Here is the latest patch and the regression tests for the temp tables >> and 2PC issue. >> Is there a way to stop and restart postmaster between 2 tests? >> >> Thanks for your feedback, >> Emmanuel > I did not get any comment on that one. > How should I proceed so that the patch integration can be considered for > 8.4? http://wiki.postgresql.org/wiki/Submitting_a_Patch -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Emmanuel Cecchet wrote: > Here is the latest patch and the regression tests for the temp tables > and 2PC issue. This fails: postgres=# begin; BEGIN postgres=# CREATE TEMPORARY TABLE temp1 (id int4); CREATE TABLE postgres=# PREPARE TRANSACTION 'foo'; PREPARE TRANSACTION postgres=# CREATE TEMPORARY TABLE temp2 (id int4); ERROR: cannot insert into frozen hashtable "accessed temp tables" I don't understand the bookkeeping of accessed and prepared temp tables in general. What's it for? The comments on preparedTempRel says that it keeps track of "accessed temporary relations that have been prepared commit but not committed yet". That's never going to work as a backend-private hash table, because there's no way to remove entries from it when the prepared transaction is committed or rolled back from another backend. What's the purpose of checking that a table is empty on prepare? I think I'd feel more comfortable with the approach of only accepting PREPARE TRANSACTIOn if the accessed temp tables have been created and destroyed in the same transaction, to avoid possibly surprising behavior when a temp table is kept locked by a prepared transaction and you try to drop it later in the sesssion, but the patch allows more than that. I guess accessing an existing ON COMMIT DELETE ROWS temp table would also be OK, but checking that there's no visible rows in thetable doesn't achieve that. I don't think you can just ignore "prepared temp relations" in findDependentObjects to avoid the lockup at backend exit. It's also used for DROP CASCADE, for example. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Hi Heikki, > Emmanuel Cecchet wrote: >> Here is the latest patch and the regression tests for the temp tables >> and 2PC issue. > > This fails: > > postgres=# begin; > BEGIN > postgres=# CREATE TEMPORARY TABLE temp1 (id int4); > CREATE TABLE > postgres=# PREPARE TRANSACTION 'foo'; > PREPARE TRANSACTION > postgres=# CREATE TEMPORARY TABLE temp2 (id int4); > ERROR: cannot insert into frozen hashtable "accessed temp tables" I will address that. > I don't understand the bookkeeping of accessed and prepared temp tables > in general. What's it for? Right now (in 8.3) the bookkeeping prevents a transaction that has used a temp table to prepare commit. As you mentioned earlier (http://archives.postgresql.org/pgsql-hackers/2008-02/msg01277.php) we should be able to allow CREATE+DROP in the same transaction. > The comments on preparedTempRel says that it keeps track of "accessed > temporary relations that have been prepared commit but not committed > yet". That's never going to work as a backend-private hash table, > because there's no way to remove entries from it when the prepared > transaction is committed or rolled back from another backend. It does not really matter since we only allow empty temp tables at prepared time. And the transaction can only be prepared locally. If the transaction is committed or rolled back from another backend, the only thing that can happen is that tables that were created in the transaction will remain in the list. They will be ignored at the next prepare since the relation will not exist anymore. Once again, the tables remaining in the list after prepare are empty. > What's the purpose of checking that a table is empty on prepare? I think > I'd feel more comfortable with the approach of only accepting PREPARE > TRANSACTIOn if the accessed temp tables have been created and destroyed > in the same transaction, to avoid possibly surprising behavior when a > temp table is kept locked by a prepared transaction and you try to drop > it later in the sesssion, but the patch allows more than that. I guess > accessing an existing ON COMMIT DELETE ROWS temp table would also be OK, Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE ROW. An empty temp table at PREPARE time would be similar to an ON COMMIT DELETE ROW table. > but checking that there's no visible rows in the table doesn't achieve > that. If the relation exist but contains no row, is it possible that the table is not empty? What would I need to do to ensure that the table is empty? > I don't think you can just ignore "prepared temp relations" in > findDependentObjects to avoid the lockup at backend exit. It's also used > for DROP CASCADE, for example. Do you mean that it will break the DROP CASCADE behavior in general, or that would break the behavior for master/child temp tables? By the way, does Postgres support child temp tables? Thanks for the feedback. I will address the problem of the frozen hash list but let me know what you think of the other potential issues. Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Emmanuel Cecchet wrote: >> What's the purpose of checking that a table is empty on prepare? I think >> I'd feel more comfortable with the approach of only accepting PREPARE >> TRANSACTIOn if the accessed temp tables have been created and destroyed >> in the same transaction, to avoid possibly surprising behavior when a >> temp table is kept locked by a prepared transaction and you try to drop >> it later in the sesssion, but the patch allows more than that. I guess >> accessing an existing ON COMMIT DELETE ROWS temp table would also be OK, > Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE ROW. > An empty temp table at PREPARE time would be similar to an ON COMMIT > DELETE ROW table. I think you'll want to check explicitly that the table is defined with ON COMMIT DELETE ROWS, instead of checking that it's empty. >> but checking that there's no visible rows in the table doesn't achieve >> that. > If the relation exist but contains no row, is it possible that the table > is not empty? What would I need to do to ensure that the table is empty? Yeah, thanks to MVCC, it's possible that the table looks empty to the transaction being prepared, using SnapshotNow, but there's some tuples that are still visible to other transactions. For example: CREATE TEMPORARY TABLE foo (id int4); INSERT INTO foo VALUES (1); begin; DELETE FROM foo; PREPARE TRANSACTION 'foo'; -- doesn't error, because the table is empty, according to SnapshotNow SELECT * FROM foo; -- Still shows the one row, because the deleting transaction hasn't committed yet. >> I don't think you can just ignore "prepared temp relations" in >> findDependentObjects to avoid the lockup at backend exit. It's also used >> for DROP CASCADE, for example. > Do you mean that it will break the DROP CASCADE behavior in general, or > that would break the behavior for master/child temp tables? For temp tables, I suppose. The hack in findDependentObjects still isn't enough, anyway. If you have a prepared transaction that created a temp table, the database doesn't shut down: $ bin/pg_ctl -D data start server starting $ LOG: database system was shut down at 2008-11-04 10:27:27 EST LOG: autovacuum launcher started LOG: database system is ready to accept connections $ bin/psql postgres -c "begin; CREATE TEMPORARY TABLE temp (id integer); PREPARE TRANSACTION 'foo';" PREPARE TRANSACTION hlinnaka@heikkilaptop:~/pgsql.fsmfork$ bin/pg_ctl -D data stop LOG: received smart shutdown request LOG: autovacuum launcher shutting down waiting for server to shut down............................................................... failed pg_ctl: server does not shut down > By the way, > does Postgres support child temp tables? Yes. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: >> Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE >> ROW. An empty temp table at PREPARE time would be similar to an ON >> COMMIT DELETE ROW table. > I think you'll want to check explicitly that the table is defined with > ON COMMIT DELETE ROWS, instead of checking that it's empty. Where can I find the field containing the CREATE options for the temp table? > Yeah, thanks to MVCC, it's possible that the table looks empty to the > transaction being prepared, using SnapshotNow, but there's some tuples > that are still visible to other transactions. For example: > > CREATE TEMPORARY TABLE foo (id int4); > INSERT INTO foo VALUES (1); > begin; > DELETE FROM foo; > PREPARE TRANSACTION 'foo'; -- doesn't error, because the table is > empty, according to SnapshotNow > SELECT * FROM foo; -- Still shows the one row, because the deleting > transaction hasn't committed yet. Is that a problem? If your transaction isolation level is not serializable the SELECT will not block and return the current snapshot. From the transaction standpoint, it is fine thatthe transaction can prepare or am I missing something? Actually, I did a test and if the temp table is created with 'on commit delete rows' option, the select blocks until the transaction is committed. This seems a normal behavior to me. >>> I don't think you can just ignore "prepared temp relations" in >>> findDependentObjects to avoid the lockup at backend exit. It's also >>> used >>> for DROP CASCADE, for example. >> Do you mean that it will break the DROP CASCADE behavior in general, >> or that would break the behavior for master/child temp tables? > > For temp tables, I suppose. I confirm that doing a drop cascade on a master temp table after a prepared transaction committed from another backend will not drop the children for now. > > The hack in findDependentObjects still isn't enough, anyway. If you > have a prepared transaction that created a temp table, the database > doesn't shut down: > > $ bin/pg_ctl -D data start > server starting > $ LOG: database system was shut down at 2008-11-04 10:27:27 EST > LOG: autovacuum launcher started > LOG: database system is ready to accept connections > > $ bin/psql postgres -c "begin; CREATE TEMPORARY TABLE temp (id > integer); PREPARE TRANSACTION 'foo';" > PREPARE TRANSACTION > hlinnaka@heikkilaptop:~/pgsql.fsmfork$ bin/pg_ctl -D data stop > LOG: received smart shutdown request > LOG: autovacuum launcher shutting down > waiting for server to shut > down............................................................... > failed > pg_ctl: server does not shut down Interesting case, if the table is created but not accessed it is not enlisted and then the shutdown does not catch this dependency. The table should be enlisted at CREATE time as well. The bookkeeping of prepared commit tables is just for the shutdown case right now. If you think it is a bad idea altogether to have session temp tables (even with delete rows on commit) that can cross commit boundaries, then we can remove that second bookkeeping and only allow temp tables that have been created withing the scope of the transaction. I fixed the hash_freeze problem but this drop cascade on temp table seems to be an issue (if anyone uses that feature). Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Hi, As I have not found yet an elegant solution to deal with the DROP CASCADE issue, here is a simpler patch that handles temp tables that are dropped at commit time. This is a good first step and we will try to elaborate further to support ON COMMIT DELETE ROWS. I have also added a compilation of the tests I have even if some are not really relevant anymore without the support for empty temp tables but we will probably reuse them later. Thanks in advance for the feedback, Emmanuel Emmanuel Cecchet wrote: > Heikki Linnakangas wrote: >>> Yes, I was trying to allow also ON COMMIT DROP and ON COMMIT DELETE >>> ROW. An empty temp table at PREPARE time would be similar to an ON >>> COMMIT DELETE ROW table. >> I think you'll want to check explicitly that the table is defined >> with ON COMMIT DELETE ROWS, instead of checking that it's empty. > Where can I find the field containing the CREATE options for the temp > table? >> Yeah, thanks to MVCC, it's possible that the table looks empty to the >> transaction being prepared, using SnapshotNow, but there's some >> tuples that are still visible to other transactions. For example: >> >> CREATE TEMPORARY TABLE foo (id int4); >> INSERT INTO foo VALUES (1); >> begin; >> DELETE FROM foo; >> PREPARE TRANSACTION 'foo'; -- doesn't error, because the table is >> empty, according to SnapshotNow >> SELECT * FROM foo; -- Still shows the one row, because the deleting >> transaction hasn't committed yet. > Is that a problem? If your transaction isolation level is not > serializable the SELECT will not block and return the current > snapshot. From the transaction standpoint, it is fine that the > transaction can prepare or am I missing something? > Actually, I did a test and if the temp table is created with 'on > commit delete rows' option, the select blocks until the transaction is > committed. This seems a normal behavior to me. >>>> I don't think you can just ignore "prepared temp relations" in >>>> findDependentObjects to avoid the lockup at backend exit. It's also >>>> used >>>> for DROP CASCADE, for example. >>> Do you mean that it will break the DROP CASCADE behavior in general, >>> or that would break the behavior for master/child temp tables? >> >> For temp tables, I suppose. > I confirm that doing a drop cascade on a master temp table after a > prepared transaction committed from another backend will not drop the > children for now. >> >> The hack in findDependentObjects still isn't enough, anyway. If you >> have a prepared transaction that created a temp table, the database >> doesn't shut down: >> >> $ bin/pg_ctl -D data start >> server starting >> $ LOG: database system was shut down at 2008-11-04 10:27:27 EST >> LOG: autovacuum launcher started >> LOG: database system is ready to accept connections >> >> $ bin/psql postgres -c "begin; CREATE TEMPORARY TABLE temp (id >> integer); PREPARE TRANSACTION 'foo';" >> PREPARE TRANSACTION >> hlinnaka@heikkilaptop:~/pgsql.fsmfork$ bin/pg_ctl -D data stop >> LOG: received smart shutdown request >> LOG: autovacuum launcher shutting down >> waiting for server to shut >> down............................................................... >> failed >> pg_ctl: server does not shut down > Interesting case, if the table is created but not accessed it is not > enlisted and then the shutdown does not catch this dependency. The > table should be enlisted at CREATE time as well. > > The bookkeeping of prepared commit tables is just for the shutdown > case right now. If you think it is a bad idea altogether to have > session temp tables (even with delete rows on commit) that can cross > commit boundaries, then we can remove that second bookkeeping and only > allow temp tables that have been created withing the scope of the > transaction. > > I fixed the hash_freeze problem but this drop cascade on temp table > seems to be an issue (if anyone uses that feature). > > Emmanuel > -- 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-HEAD Index: src/backend/access/heap/heapam.c =================================================================== RCS file: /root/cvsrepo/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.268 diff -u -r1.268 heapam.c --- src/backend/access/heap/heapam.c 31 Oct 2008 19:40:26 -0000 1.268 +++ src/backend/access/heap/heapam.c 7 Nov 2008 23:09:11 -0000 @@ -878,7 +878,7 @@ /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -926,7 +926,7 @@ /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) - MyXactAccessedTempRel = true; + enlistRelationIdFor2PCChecks(relationId); pgstat_initstats(r); @@ -976,7 +976,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.266 diff -u -r1.266 xact.c --- src/backend/access/transam/xact.c 20 Oct 2008 19:18:18 -0000 1.266 +++ src/backend/access/transam/xact.c 7 Nov 2008 23:09:11 -0000 @@ -65,13 +65,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 @@ -209,6 +202,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 */ @@ -250,6 +246,7 @@ SubTransactionId mySubid, SubTransactionId parentSubid); static void CleanupTransaction(void); +static void CleanupAccessedTempRel(void); static void CommitTransaction(void); static TransactionId RecordTransactionAbort(bool isSubXact); static void StartTransaction(void); @@ -623,7 +620,7 @@ /* Propagate new command ID into static snapshots */ SnapshotSetCommandId(currentCommandId); - + /* * Make any catalog changes done by the just-completed command * visible in the local syscache. We obviously don't need to do @@ -1110,10 +1107,10 @@ */ if (s->parent->childXids == NULL) new_childXids = - MemoryContextAlloc(TopTransactionContext, + MemoryContextAlloc(TopTransactionContext, new_maxChildXids * sizeof(TransactionId)); else - new_childXids = repalloc(s->parent->childXids, + new_childXids = repalloc(s->parent->childXids, new_maxChildXids * sizeof(TransactionId)); s->parent->childXids = new_childXids; @@ -1466,7 +1463,6 @@ XactIsoLevel = DefaultXactIsoLevel; XactReadOnly = DefaultXactReadOnly; forceSyncCommit = false; - MyXactAccessedTempRel = false; /* * reinitialize within-transaction counters @@ -1715,6 +1711,8 @@ AtCommit_Memory(); + CleanupAccessedTempRel(); + s->transactionId = InvalidTransactionId; s->subTransactionId = InvalidSubTransactionId; s->nestingLevel = 0; @@ -1732,6 +1730,50 @@ 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) +{ + Oid *tid; + + /* + * 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 + tid = hash_search(accessedTempRel, &relationId, HASH_ENTER, NULL); + *tid = relationId; +} + +/* + * Cleanup the list of prepared temp tables that were accessed during this transaction. + */ +static void CleanupAccessedTempRel(void) +{ + if (accessedTempRel != NULL) + { + hash_destroy(accessedTempRel); + accessedTempRel = NULL; + } +} /* * PrepareTransaction @@ -1808,14 +1850,37 @@ * 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 throw an error + relation_close(tempTable, NoLock); + 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 notdropped at commit time"))); + } + } + + hash_destroy(accessedTempRel); + accessedTempRel = NULL; + } /* Prevent cancel/die interrupt while cleaning up */ HOLD_INTERRUPTS(); @@ -2106,6 +2171,8 @@ elog(FATAL, "CleanupTransaction: unexpected state %s", TransStateAsString(s->state)); + CleanupAccessedTempRel(); + /* * do abort cleanup processing */ Index: src/include/access/xact.h =================================================================== RCS file: /root/cvsrepo/pgsql/src/include/access/xact.h,v retrieving revision 1.95 diff -u -r1.95 xact.h --- src/include/access/xact.h 11 Aug 2008 11:05:11 -0000 1.95 +++ src/include/access/xact.h 7 Nov 2008 23:09:11 -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 ### Eclipse Workspace Patch 1.0 #P Postgres-HEAD Index: src/test/regress/parallel_schedule =================================================================== RCS file: /root/cvsrepo/pgsql/src/test/regress/parallel_schedule,v retrieving revision 1.50 diff -u -r1.50 parallel_schedule --- src/test/regress/parallel_schedule 31 Oct 2008 09:17:16 -0000 1.50 +++ src/test/regress/parallel_schedule 7 Nov 2008 23:13:53 -0000 @@ -90,3 +90,19 @@ # 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 +# The following tests must be executing in sequence, +# do not alter the order nor try to execute them in parallel +test: 2pc_temp_table_prepared_not_committed +test: 2pc_temp_table_commit_prepared +# This test must be run last to check if the database properly +# shutdowns with a prepared transaction that is not committed +test: 2pc_temp_table_prepared_not_committed + \ No newline at end of file Index: src/test/regress/serial_schedule =================================================================== RCS file: /root/cvsrepo/pgsql/src/test/regress/serial_schedule,v retrieving revision 1.47 diff -u -r1.47 serial_schedule --- src/test/regress/serial_schedule 31 Oct 2008 09:17:16 -0000 1.47 +++ src/test/regress/serial_schedule 7 Nov 2008 23:13:53 -0000 @@ -119,3 +119,20 @@ test: xml test: stats test: tablespace +test: 2pc_persistent_table +test: 2pc_on_delete_rows_transaction +test: 2pc_on_commit_drop +test: 2pc_temp_table_transaction +test: 2pc_temp_table_rollback +test: 2pc_temp_table_savepoint +test: 2pc_dirty_buffer_check +test: 2pc_temp_table_session +test: 2pc_on_delete_rows_session +test: 2pc_temp_table_failure +# The following tests must be executing in sequence, +# do not alter the order nor try to execute them in parallel +test: 2pc_temp_table_prepared_not_committed +test: 2pc_temp_table_commit_prepared +# This test must be run last to check if the database properly +# shutdowns with a prepared transaction that is not committed +test: 2pc_temp_table_prepared_not_committed Index: src/test/regress/expected/temp.out =================================================================== RCS file: /root/cvsrepo/pgsql/src/test/regress/expected/temp.out,v retrieving revision 1.15 diff -u -r1.15 temp.out --- src/test/regress/expected/temp.out 1 Sep 2008 20:42:46 -0000 1.15 +++ src/test/regress/expected/temp.out 7 Nov 2008 23:13:53 -0000 @@ -1,3 +1,13 @@ +create temp table master (x int); +create temp table child () inherits (master); +insert into master values (1); +insert into child values (2); +drop table master cascade; +NOTICE: drop cascades to table child +select * from child; +ERROR: relation "child" does not exist +LINE 1: select * from child; + ^ -- -- TEMP -- Test temp relations and indexes Index: src/test/regress/results/2pc_temp_table_transaction.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_transaction.out diff -N src/test/regress/results/2pc_temp_table_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_temp_table_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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'; Index: src/test/regress/results/2pc_temp_table_prepared_not_committed.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_prepared_not_committed.out diff -N src/test/regress/results/2pc_temp_table_prepared_not_committed.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_temp_table_prepared_not_committed.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,23 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- Session-scope temp table use case but exit after prepared +-- see 2pc_temp_table_commit_prepared.sql for committing that transaction +create temp table foo1(x int); +insert into foo1 values(1); +begin; +delete from foo1; +prepare transaction 'preparedNonCommittedFoo1'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +select * from foo1; + x +--- + 1 +(1 row) + +create temp table foo2(x int); +begin; +insert into foo2 values(1); +delete from foo2; +prepare transaction 'preparedNonCommittedFoo2'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time 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_transaction.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_transaction.out diff -N src/test/regress/expected/2pc_temp_table_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- 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'; Index: src/test/regress/results/2pc_temp_table_commit_prepared.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_commit_prepared.out diff -N src/test/regress/results/2pc_temp_table_commit_prepared.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_temp_table_commit_prepared.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,14 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- This test has to be called in sequence after 2pc_temp_table_prepared_not_committed.sql +commit prepared 'preparedNonCommittedFoo1'; +ERROR: prepared transaction with identifier "preparedNonCommittedFoo1" does not exist +-- The table should not exist anymore in this session +drop table foo1; +ERROR: table "foo1" does not exist +commit prepared 'preparedNonCommittedFoo2'; +ERROR: prepared transaction with identifier "preparedNonCommittedFoo2" does not exist +-- The table should not exist anymore in this session +drop table foo2; +ERROR: table "foo2" does not exist Index: src/test/regress/expected/2pc_on_delete_rows_transaction.out =================================================================== RCS file: src/test/regress/expected/2pc_on_delete_rows_transaction.out diff -N src/test/regress/expected/2pc_on_delete_rows_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_on_delete_rows_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,10 @@ +-- 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'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +commit prepared 'onCommitDeleteRowsTempTableShouldSucceed'; +ERROR: prepared transaction with identifier "onCommitDeleteRowsTempTableShouldSucceed" does not exist +drop table foo; +ERROR: table "foo" does not exist Index: src/test/regress/results/2pc_on_delete_rows_transaction.out =================================================================== RCS file: src/test/regress/results/2pc_on_delete_rows_transaction.out diff -N src/test/regress/results/2pc_on_delete_rows_transaction.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_on_delete_rows_transaction.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,10 @@ +-- 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'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +commit prepared 'onCommitDeleteRowsTempTableShouldSucceed'; +ERROR: prepared transaction with identifier "onCommitDeleteRowsTempTableShouldSucceed" does not exist +drop table foo; +ERROR: table "foo" does not exist Index: src/test/regress/expected/2pc_persistent_table.out =================================================================== RCS file: src/test/regress/expected/2pc_persistent_table.out diff -N src/test/regress/expected/2pc_persistent_table.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_persistent_table.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,11 @@ +-- 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_session.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_session.out diff -N src/test/regress/expected/2pc_temp_table_session.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_session.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,10 @@ +-- Session-scope temp table use case +create temp table foo(x int); +begin; +insert into foo values(1); +delete from foo; +prepare transaction 'dropTempTableShouldSucceed'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +commit prepared 'dropTempTableShouldSucceed'; +ERROR: prepared transaction with identifier "dropTempTableShouldSucceed" does not exist +drop table foo; Index: src/test/regress/results/2pc_on_commit_drop.out =================================================================== RCS file: src/test/regress/results/2pc_on_commit_drop.out diff -N src/test/regress/results/2pc_on_commit_drop.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_on_commit_drop.out 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'; Index: src/test/regress/expected/2pc_temp_table_commit_prepared.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_commit_prepared.out diff -N src/test/regress/expected/2pc_temp_table_commit_prepared.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_commit_prepared.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,14 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- This test has to be called in sequence after 2pc_temp_table_prepared_not_committed.sql +commit prepared 'preparedNonCommittedFoo1'; +ERROR: prepared transaction with identifier "preparedNonCommittedFoo1" does not exist +-- The table should not exist anymore in this session +drop table foo1; +ERROR: table "foo1" does not exist +commit prepared 'preparedNonCommittedFoo2'; +ERROR: prepared transaction with identifier "preparedNonCommittedFoo2" does not exist +-- The table should not exist anymore in this session +drop table foo2; +ERROR: table "foo2" does not exist 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 dropped at commit time +commit prepared 'existingTempTableShouldFail'; +ERROR: prepared transaction with identifier "existingTempTableShouldFail" does not exist 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,9 @@ +-- 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'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +commit prepared 'onCommitDeleteRowsTempTableShouldSucceed'; +ERROR: prepared transaction with identifier "onCommitDeleteRowsTempTableShouldSucceed" does not exist +drop table foo; Index: src/test/regress/results/2pc_dirty_buffer_check.out =================================================================== RCS file: src/test/regress/results/2pc_dirty_buffer_check.out diff -N src/test/regress/results/2pc_dirty_buffer_check.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_dirty_buffer_check.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,52 @@ +-- 2PC Dirty buffer check +begin; +create temp table foo(a int, b int, c int) on commit drop; +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; +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; Index: src/test/regress/results/2pc_temp_table_rollback.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_rollback.out diff -N src/test/regress/results/2pc_temp_table_rollback.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_temp_table_rollback.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,7 @@ +-- Rollback prepared +BEGIN; +CREATE TEMP TABLE foo(bar int4); +PREPARE TRANSACTION 'rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +ROLLBACK PREPARED 'rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php'; +ERROR: prepared transaction with identifier "rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php"does not exist Index: src/test/regress/results/temp.out =================================================================== RCS file: src/test/regress/results/temp.out diff -N src/test/regress/results/temp.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/temp.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,213 @@ +create temp table master (x int); +create temp table child () inherits (master); +insert into master values (1); +insert into child values (2); +drop table master cascade; +NOTICE: drop cascades to table child +select * from child; +ERROR: relation "child" does not exist +LINE 1: select * from child; + ^ +-- +-- TEMP +-- Test temp relations and indexes +-- +-- test temp table/index masking +CREATE TABLE temptest(col int); +CREATE INDEX i_temptest ON temptest(col); +CREATE TEMP TABLE temptest(tcol int); +CREATE INDEX i_temptest ON temptest(tcol); +SELECT * FROM temptest; + tcol +------ +(0 rows) + +DROP INDEX i_temptest; +DROP TABLE temptest; +SELECT * FROM temptest; + col +----- +(0 rows) + +DROP INDEX i_temptest; +DROP TABLE temptest; +-- test temp table selects +CREATE TABLE temptest(col int); +INSERT INTO temptest VALUES (1); +CREATE TEMP TABLE temptest(tcol float); +INSERT INTO temptest VALUES (2.1); +SELECT * FROM temptest; + tcol +------ + 2.1 +(1 row) + +DROP TABLE temptest; +SELECT * FROM temptest; + col +----- + 1 +(1 row) + +DROP TABLE temptest; +-- test temp table deletion +CREATE TEMP TABLE temptest(col int); +\c +SELECT * FROM temptest; +ERROR: relation "temptest" does not exist +LINE 1: SELECT * FROM temptest; + ^ +-- Test ON COMMIT DELETE ROWS +CREATE TEMP TABLE temptest(col int) ON COMMIT DELETE ROWS; +BEGIN; +INSERT INTO temptest VALUES (1); +INSERT INTO temptest VALUES (2); +SELECT * FROM temptest; + col +----- + 1 + 2 +(2 rows) + +COMMIT; +SELECT * FROM temptest; + col +----- +(0 rows) + +DROP TABLE temptest; +BEGIN; +CREATE TEMP TABLE temptest(col) ON COMMIT DELETE ROWS AS SELECT 1; +SELECT * FROM temptest; + col +----- + 1 +(1 row) + +COMMIT; +SELECT * FROM temptest; + col +----- +(0 rows) + +DROP TABLE temptest; +-- Test ON COMMIT DROP +BEGIN; +CREATE TEMP TABLE temptest(col int) ON COMMIT DROP; +INSERT INTO temptest VALUES (1); +INSERT INTO temptest VALUES (2); +SELECT * FROM temptest; + col +----- + 1 + 2 +(2 rows) + +COMMIT; +SELECT * FROM temptest; +ERROR: relation "temptest" does not exist +LINE 1: SELECT * FROM temptest; + ^ +BEGIN; +CREATE TEMP TABLE temptest(col) ON COMMIT DROP AS SELECT 1; +SELECT * FROM temptest; + col +----- + 1 +(1 row) + +COMMIT; +SELECT * FROM temptest; +ERROR: relation "temptest" does not exist +LINE 1: SELECT * FROM temptest; + ^ +-- ON COMMIT is only allowed for TEMP +CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS; +ERROR: ON COMMIT can only be used on temporary tables +CREATE TABLE temptest(col) ON COMMIT DELETE ROWS AS SELECT 1; +ERROR: ON COMMIT can only be used on temporary tables +-- Test foreign keys +BEGIN; +CREATE TEMP TABLE temptest1(col int PRIMARY KEY); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "temptest1_pkey" for table "temptest1" +CREATE TEMP TABLE temptest2(col int REFERENCES temptest1) + ON COMMIT DELETE ROWS; +INSERT INTO temptest1 VALUES (1); +INSERT INTO temptest2 VALUES (1); +COMMIT; +SELECT * FROM temptest1; + col +----- + 1 +(1 row) + +SELECT * FROM temptest2; + col +----- +(0 rows) + +BEGIN; +CREATE TEMP TABLE temptest3(col int PRIMARY KEY) ON COMMIT DELETE ROWS; +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "temptest3_pkey" for table "temptest3" +CREATE TEMP TABLE temptest4(col int REFERENCES temptest3); +COMMIT; +ERROR: unsupported ON COMMIT and foreign key combination +DETAIL: Table "temptest4" references "temptest3", but they do not have the same ON COMMIT setting. +-- Test manipulation of temp schema's placement in search path +create table public.whereami (f1 text); +insert into public.whereami values ('public'); +create temp table whereami (f1 text); +insert into whereami values ('temp'); +create function public.whoami() returns text + as $$select 'public'::text$$ language sql; +create function pg_temp.whoami() returns text + as $$select 'temp'::text$$ language sql; +-- default should have pg_temp implicitly first, but only for tables +select * from whereami; + f1 +------ + temp +(1 row) + +select whoami(); + whoami +-------- + public +(1 row) + +-- can list temp first explicitly, but it still doesn't affect functions +set search_path = pg_temp, public; +select * from whereami; + f1 +------ + temp +(1 row) + +select whoami(); + whoami +-------- + public +(1 row) + +-- or put it last for security +set search_path = public, pg_temp; +select * from whereami; + f1 +-------- + public +(1 row) + +select whoami(); + whoami +-------- + public +(1 row) + +-- you can invoke a temp function explicitly, though +select pg_temp.whoami(); + whoami +-------- + temp +(1 row) + +drop table public.whereami; Index: src/test/regress/results/2pc_on_delete_rows_session.out =================================================================== RCS file: src/test/regress/results/2pc_on_delete_rows_session.out diff -N src/test/regress/results/2pc_on_delete_rows_session.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_on_delete_rows_session.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,9 @@ +-- 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'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +commit prepared 'onCommitDeleteRowsTempTableShouldSucceed'; +ERROR: prepared transaction with identifier "onCommitDeleteRowsTempTableShouldSucceed" does not exist +drop table foo; Index: src/test/regress/expected/2pc_temp_table_prepared_not_committed.out =================================================================== RCS file: src/test/regress/expected/2pc_temp_table_prepared_not_committed.out diff -N src/test/regress/expected/2pc_temp_table_prepared_not_committed.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_temp_table_prepared_not_committed.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,23 @@ +-- The purpose of this test is to test the proper termination of +-- a session with a prepared transaction that has accessed a temp table +-- +-- Session-scope temp table use case but exit after prepared +-- see 2pc_temp_table_commit_prepared.sql for committing that transaction +create temp table foo1(x int); +insert into foo1 values(1); +begin; +delete from foo1; +prepare transaction 'preparedNonCommittedFoo1'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +select * from foo1; + x +--- + 1 +(1 row) + +create temp table foo2(x int); +begin; +insert into foo2 values(1); +delete from foo2; +prepare transaction 'preparedNonCommittedFoo2'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time Index: src/test/regress/results/2pc_temp_table_savepoint.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_savepoint.out diff -N src/test/regress/results/2pc_temp_table_savepoint.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/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_on_commit_drop.out =================================================================== RCS file: src/test/regress/expected/2pc_on_commit_drop.out diff -N src/test/regress/expected/2pc_on_commit_drop.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_on_commit_drop.out 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'; Index: src/test/regress/expected/2pc_dirty_buffer_check.out =================================================================== RCS file: src/test/regress/expected/2pc_dirty_buffer_check.out diff -N src/test/regress/expected/2pc_dirty_buffer_check.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/expected/2pc_dirty_buffer_check.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,52 @@ +-- 2PC Dirty buffer check +begin; +create temp table foo(a int, b int, c int) on commit drop; +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; +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; Index: src/test/regress/results/2pc_persistent_table.out =================================================================== RCS file: src/test/regress/results/2pc_persistent_table.out diff -N src/test/regress/results/2pc_persistent_table.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_persistent_table.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,11 @@ +-- 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/results/2pc_temp_table_session.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_session.out diff -N src/test/regress/results/2pc_temp_table_session.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/2pc_temp_table_session.out 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,10 @@ +-- Session-scope temp table use case +create temp table foo(x int); +begin; +insert into foo values(1); +delete from foo; +prepare transaction 'dropTempTableShouldSucceed'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +commit prepared 'dropTempTableShouldSucceed'; +ERROR: prepared transaction with identifier "dropTempTableShouldSucceed" does not exist +drop table foo; Index: src/test/regress/results/2pc_temp_table_failure.out =================================================================== RCS file: src/test/regress/results/2pc_temp_table_failure.out diff -N src/test/regress/results/2pc_temp_table_failure.out --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ src/test/regress/results/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 dropped at commit time +commit prepared 'existingTempTableShouldFail'; +ERROR: prepared transaction with identifier "existingTempTableShouldFail" does not exist 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,7 @@ +-- Rollback prepared +BEGIN; +CREATE TEMP TABLE foo(bar int4); +PREPARE TRANSACTION 'rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php'; +ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not dropped at commit time +ROLLBACK PREPARED 'rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php'; +ERROR: prepared transaction with identifier "rollbackTestCase_archives.postgresql.org_pgsql-hackers_2008-03_msg00017.php"does not exist
Emmanuel Cecchet wrote: > As I have not found yet an elegant solution to deal with the DROP > CASCADE issue, here is a simpler patch that handles temp tables that are > dropped at commit time. This is a good first step and we will try to > elaborate further to support ON COMMIT DELETE ROWS. The problem with stopping the postmaster seems to still be there.. All the problems are centered around locking. We need to address that and decide what is sane locking behavior wrt. temp tables and 2PC. First, there's the case where a temp table is created and dropped in the same transaction. It seems perfectly sane to me to simply drop all locks on the dropped table at PREPARE TRANSACTION. Does anyone see a problem with that? If not, we might as well do that for all tables, not just temporary ones. It seems just as safe for non-temporary tables. Secondly, there's the case of accessing a ON COMMIT DELETE ROWS table. There too, could we simply drop the locks at PREPARE TRANSACTION? I can't immediately see anything wrong with that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Hi Heikki, > Emmanuel Cecchet wrote: >> As I have not found yet an elegant solution to deal with the DROP >> CASCADE issue, here is a simpler patch that handles temp tables that >> are dropped at commit time. This is a good first step and we will try >> to elaborate further to support ON COMMIT DELETE ROWS. > > The problem with stopping the postmaster seems to still be there.. > > All the problems are centered around locking. We need to address that > and decide what is sane locking behavior wrt. temp tables and 2PC. > > First, there's the case where a temp table is created and dropped in > the same transaction. It seems perfectly sane to me to simply drop all > locks on the dropped table at PREPARE TRANSACTION. Does anyone see a > problem with that? If not, we might as well do that for all tables, > not just temporary ones. It seems just as safe for non-temporary tables. This seems good to me. Any access to the table after PREPARE TRANSACTION but before COMMIT on that backend would return a relation not found which is what we expect. For a regular table, I don't know if that makes a difference if the prepared transaction rollbacks? > Secondly, there's the case of accessing a ON COMMIT DELETE ROWS table. > There too, could we simply drop the locks at PREPARE TRANSACTION? I > can't immediately see anything wrong with that. As there is no data anyway, I don't think the locks are going to change anything. But in the most recent stripped-down version of the patch, on delete rows is no more supported, I only allow on commit drop. I did not find the flag to see if a temp table was created with the on delete rows option. Do you want me to look at the locking code or will you have time to do it? Hints will be welcome if you want me to do it. Thanks, Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Emmanuel Cecchet wrote: >> Emmanuel Cecchet wrote: >>> As I have not found yet an elegant solution to deal with the DROP >>> CASCADE issue, here is a simpler patch that handles temp tables that >>> are dropped at commit time. This is a good first step and we will try >>> to elaborate further to support ON COMMIT DELETE ROWS. >> >> The problem with stopping the postmaster seems to still be there.. >> >> All the problems are centered around locking. We need to address that >> and decide what is sane locking behavior wrt. temp tables and 2PC. >> >> First, there's the case where a temp table is created and dropped in >> the same transaction. It seems perfectly sane to me to simply drop all >> locks on the dropped table at PREPARE TRANSACTION. Does anyone see a >> problem with that? If not, we might as well do that for all tables, >> not just temporary ones. It seems just as safe for non-temporary tables. > This seems good to me. Any access to the table after PREPARE TRANSACTION > but before COMMIT on that backend would return a relation not found > which is what we expect. For a regular table, I don't know if that > makes a difference if the prepared transaction rollbacks? I don't think there's any difference with temp tables and regular ones from locking point of view. >> Secondly, there's the case of accessing a ON COMMIT DELETE ROWS table. >> There too, could we simply drop the locks at PREPARE TRANSACTION? I >> can't immediately see anything wrong with that. > As there is no data anyway, I don't think the locks are going to change > anything. But in the most recent stripped-down version of the patch, on > delete rows is no more supported, I only allow on commit drop. I did not > find the flag to see if a temp table was created with the on delete rows > option. Hmm. I think we can use the on_commits list in tablecmds.c for that. > Do you want me to look at the locking code or will you have time to do > it? Hints will be welcome if you want me to do it. I can give it a shot for change. Attached is a patch that allows the ON COMMIT DELETE ROWS case. The beef of the patch is: - An entry is made into the on_commits list in tablecmds.c for all temp tables, even if there's no ON COMMIT action - There's a new function, check_prepare_safe_temp_table(Oid relid) in tablecmds.c, that uses the on_commits list to determine if the access to the given relation is "PREPARE-safe". That is, it's not a temp table, or it's an access to an ON COMMIT DELETE ROWS temp table and the temp table wasn't created or dropped in the same transaction. - MyXactMadeTempRelUpdate variable is gone. The check is driven from the lock manager again, like it was in 8.1, by calling the new check_prepare_sage_temp_table function for all relation locks in AtPrepare_Locks(). - changed the on_commits linked list in tablecmds.c into a hash table for performance Somehow this feels pretty baroque, though. Perhaps a better approach would be to add a new AtPrepare_OnCommitActions function to tablecmds.c, that gets called before AtPrepare_Locks. It would scan through the on_commits list, and release all locks for the "PREPARE-safe" temp tables, and throw the error if necessary. I'll try that next. BTW, there's a very relevant thread here: http://archives.postgresql.org/pgsql-hackers/2008-03/msg00063.php if you haven't read it already. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 33d5fab..b94e2bd 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -876,10 +876,6 @@ 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; @@ -924,10 +920,6 @@ 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; @@ -974,10 +966,6 @@ 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; diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index b7d1285..c859874 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -67,14 +67,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 */ typedef enum TransState @@ -1467,7 +1459,6 @@ StartTransaction(void) XactIsoLevel = DefaultXactIsoLevel; XactReadOnly = DefaultXactReadOnly; forceSyncCommit = false; - MyXactAccessedTempRel = false; /* * reinitialize within-transaction counters @@ -1798,26 +1789,6 @@ 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(); diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index afc6977..99e941b 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -39,6 +39,7 @@ #include "catalog/heap.h" #include "catalog/index.h" #include "catalog/indexing.h" +#include "catalog/namespace.h" #include "catalog/pg_attrdef.h" #include "catalog/pg_constraint.h" #include "catalog/pg_inherits.h" @@ -1064,7 +1065,7 @@ heap_create_with_catalog(const char *relname, /* * If there's a special on-commit action, remember it */ - if (oncommit != ONCOMMIT_NOOP) + if (isTempOrToastNamespace(relnamespace)) register_on_commit_action(relid, oncommit); /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6559470..f201e0d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -96,7 +96,7 @@ typedef struct OnCommitItem SubTransactionId deleting_subid; } OnCommitItem; -static List *on_commits = NIL; +static HTAB *on_commits = NULL; /* @@ -7575,26 +7575,38 @@ void register_on_commit_action(Oid relid, OnCommitAction action) { OnCommitItem *oc; - MemoryContext oldcxt; + bool found; /* - * We needn't bother registering the relation unless there is an ON COMMIT - * action we need to take. + * We need to have an entry for all temporary tables, even for + * no-op actions, see check_prepare_safe_temp_table(). */ - if (action == ONCOMMIT_NOOP || action == ONCOMMIT_PRESERVE_ROWS) - return; - oldcxt = MemoryContextSwitchTo(CacheMemoryContext); + if (on_commits == NULL) + { + HASHCTL hash_ctl; + + /* Initialize hash tables used to track update chains */ + memset(&hash_ctl, 0, sizeof(hash_ctl)); + hash_ctl.keysize = sizeof(Oid); + hash_ctl.entrysize = sizeof(OnCommitItem); + hash_ctl.hcxt = CacheMemoryContext; + hash_ctl.hash = oid_hash; + + on_commits = hash_create("On commit actions", + 4, /* small initial size to make scans fast */ + &hash_ctl, + HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT); + } + + oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_ENTER, &found); + if (found) + elog(ERROR, "relid already in on commit action table"); - oc = (OnCommitItem *) palloc(sizeof(OnCommitItem)); oc->relid = relid; oc->oncommit = action; oc->creating_subid = GetCurrentSubTransactionId(); oc->deleting_subid = InvalidSubTransactionId; - - on_commits = lcons(oc, on_commits); - - MemoryContextSwitchTo(oldcxt); } /* @@ -7605,17 +7617,90 @@ register_on_commit_action(Oid relid, OnCommitAction action) void remove_on_commit_action(Oid relid) { - ListCell *l; + OnCommitItem *oc; + + if (on_commits == NULL) + return; - foreach(l, on_commits) + oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_FIND, NULL); + + if (oc != NULL) + oc->deleting_subid = GetCurrentSubTransactionId(); +} + +/* + * 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. + * + * However, since PostgreSQL 8.4, we do allow PREPARE TRANSATION if the + * transaction has accessed a temporary table with ON COMMIT DELETE ROWS + * action, as long as the transaction hasn't created or dropped one. We + * work around the lock problem by releasing the locks early, in the PREPARE + * phase. That doesn't violate the two-phase locking protocol (not to be + * confused with two-phase commit!), as the lock would be released at the + * 2nd phase commit or rollback anyway, and the transaction won't acquire + * any more locks after PREPARE. + * + * This function is called by the lock manager in the PREPARE phase, to + * determine if the given relation is an ON COMMIT DELETE ROWS temporary + * table, and the lock manager should drop locks early. Returns 'true', if + * it is, and 'false' if the table is not a temporary table. If the table + * is a temporary table, but it's not safe for PREPARE TRANSACTION, an error + * is thrown. + * + * The reason why we only do that for ON COMMIT DELETE ROWS tables, is that + * VACUUM expects that all xids in the table are finished. XXX That's not + * strictly true, it can deal with them but prints messages to log. Is there + * any other code that could be confused by that? + * + * XXX In principle this could be relaxed to allow some other useful special + * cases, such as a temp table created and dropped all within the + * transaction. We would need to at least drop the local buffers, however. + */ +bool +check_prepare_safe_temp_table(Oid relid) +{ + OnCommitItem *oc; + + if (on_commits == NULL) + return false; /* No temp tables */ + + oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_FIND, NULL); + + if (oc == NULL) + return false; /* Not a temp table */ + + /* + * Allow access to ON COMMIT DELETE ROWS temporary tables, + * that have not been created or dropped in this transaction. + */ + if (oc->oncommit == ONCOMMIT_DELETE_ROWS && + oc->creating_subid == InvalidSubTransactionId && + oc->deleting_subid == InvalidSubTransactionId) { - OnCommitItem *oc = (OnCommitItem *) lfirst(l); + return true; + } + else + { + /* Give appropriate error message */ + if (oc->creating_subid != InvalidSubTransactionId || + oc->deleting_subid != InvalidSubTransactionId) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot PREPARE a transaction that has created or dropped a temporary table"))); + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot PREPARE a transaction that has accessed a temporary table not defined with ON COMMITDELETE ROWS"))); - if (oc->relid == relid) - { - oc->deleting_subid = GetCurrentSubTransactionId(); - break; - } + return false; /* keep compiler quite */ } } @@ -7628,19 +7713,24 @@ remove_on_commit_action(Oid relid) void PreCommit_on_commit_actions(void) { - ListCell *l; + HASH_SEQ_STATUS seq_status; List *oids_to_truncate = NIL; + OnCommitItem *oc; - foreach(l, on_commits) - { - OnCommitItem *oc = (OnCommitItem *) lfirst(l); - - /* Ignore entry if already dropped in this xact */ - if (oc->deleting_subid != InvalidSubTransactionId) - continue; + if (on_commits == NULL) + return; - switch (oc->oncommit) + hash_seq_init(&seq_status, on_commits); + PG_TRY(); + { + while ((oc = hash_seq_search(&seq_status)) != NULL) { + /* Ignore entry if already dropped in this xact */ + if (oc->deleting_subid != InvalidSubTransactionId) + continue; + + switch (oc->oncommit) + { case ONCOMMIT_NOOP: case ONCOMMIT_PRESERVE_ROWS: /* Do nothing (there shouldn't be such entries, actually) */ @@ -7665,8 +7755,15 @@ PreCommit_on_commit_actions(void) Assert(oc->deleting_subid != InvalidSubTransactionId); break; } + } } } + PG_CATCH(); + { + hash_seq_term(&seq_status); + } + PG_END_TRY(); + if (oids_to_truncate != NIL) { heap_truncate(oids_to_truncate); @@ -7685,36 +7782,36 @@ PreCommit_on_commit_actions(void) void AtEOXact_on_commit_actions(bool isCommit) { - ListCell *cur_item; - ListCell *prev_item; + HASH_SEQ_STATUS seq_status; + OnCommitItem *oc; - prev_item = NULL; - cur_item = list_head(on_commits); + if (on_commits == NULL) + return; - while (cur_item != NULL) + hash_seq_init(&seq_status, on_commits); + PG_TRY(); { - OnCommitItem *oc = (OnCommitItem *) lfirst(cur_item); - - if (isCommit ? oc->deleting_subid != InvalidSubTransactionId : - oc->creating_subid != InvalidSubTransactionId) + while ((oc = hash_seq_search(&seq_status)) != NULL) { - /* cur_item must be removed */ - on_commits = list_delete_cell(on_commits, cur_item, prev_item); - pfree(oc); - if (prev_item) - cur_item = lnext(prev_item); + if (isCommit ? oc->deleting_subid != InvalidSubTransactionId : + oc->creating_subid != InvalidSubTransactionId) + { + /* current item must be removed */ + hash_search(on_commits, &oc->relid, HASH_REMOVE, NULL); + } else - cur_item = list_head(on_commits); - } - else - { - /* cur_item must be preserved */ - oc->creating_subid = InvalidSubTransactionId; - oc->deleting_subid = InvalidSubTransactionId; - prev_item = cur_item; - cur_item = lnext(prev_item); + { + /* current item must be preserved */ + oc->creating_subid = InvalidSubTransactionId; + oc->deleting_subid = InvalidSubTransactionId; + } } } + PG_CATCH(); + { + hash_seq_term(&seq_status); + } + PG_END_TRY(); } /* @@ -7728,35 +7825,35 @@ void AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid, SubTransactionId parentSubid) { - ListCell *cur_item; - ListCell *prev_item; + HASH_SEQ_STATUS seq_status; + OnCommitItem *oc; - prev_item = NULL; - cur_item = list_head(on_commits); + if (on_commits == NULL) + return; - while (cur_item != NULL) + hash_seq_init(&seq_status, on_commits); + PG_TRY(); { - OnCommitItem *oc = (OnCommitItem *) lfirst(cur_item); - - if (!isCommit && oc->creating_subid == mySubid) + while ((oc = hash_seq_search(&seq_status)) != NULL) { - /* cur_item must be removed */ - on_commits = list_delete_cell(on_commits, cur_item, prev_item); - pfree(oc); - if (prev_item) - cur_item = lnext(prev_item); + if (!isCommit && oc->creating_subid == mySubid) + { + /* current item must be removed */ + hash_search(on_commits, &oc->relid, HASH_REMOVE, NULL); + } else - cur_item = list_head(on_commits); - } - else - { - /* cur_item must be preserved */ - if (oc->creating_subid == mySubid) - oc->creating_subid = parentSubid; - if (oc->deleting_subid == mySubid) - oc->deleting_subid = isCommit ? parentSubid : InvalidSubTransactionId; - prev_item = cur_item; - cur_item = lnext(prev_item); + { + /* current item must be preserved */ + if (oc->creating_subid == mySubid) + oc->creating_subid = parentSubid; + if (oc->deleting_subid == mySubid) + oc->deleting_subid = isCommit ? parentSubid : InvalidSubTransactionId; + } } } + PG_CATCH(); + { + hash_seq_term(&seq_status); + } + PG_END_TRY(); } diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index ccc6562..eb7d70f 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -35,6 +35,7 @@ #include "access/transam.h" #include "access/twophase.h" #include "access/twophase_rmgr.h" +#include "commands/tablecmds.h" #include "miscadmin.h" #include "pg_trace.h" #include "pgstat.h" @@ -1863,6 +1864,17 @@ AtPrepare_Locks(void) if (locallock->nLocks <= 0) continue; + /* + * Locks on ON COMMIT DELETE ROWS temp tables are released in + * PREPARE TRANSACTION phase + */ + if (locallock->tag.lock.locktag_type == LOCKTAG_RELATION) + { + Oid relid = locallock->tag.lock.locktag_field2; + if (check_prepare_safe_temp_table(relid)) + continue; + } + /* Scan to verify there are no session locks */ for (i = locallock->numLockOwners - 1; i >= 0; i--) { @@ -1944,6 +1956,17 @@ PostPrepare_Locks(TransactionId xid) if (locallock->tag.lock.locktag_type == LOCKTAG_VIRTUALTRANSACTION) continue; + /* + * Locks on ON COMMIT DELETE ROWS temp tables are released in + * PREPARE TRANSACTION phase + */ + if (locallock->tag.lock.locktag_type == LOCKTAG_RELATION) + { + Oid relid = locallock->tag.lock.locktag_field2; + if (check_prepare_safe_temp_table(relid)) + continue; + } + /* We already checked there are no session locks */ /* Mark the proclock to show we need to release this lockmode */ @@ -1993,6 +2016,17 @@ PostPrepare_Locks(TransactionId xid) if (lock->tag.locktag_type == LOCKTAG_VIRTUALTRANSACTION) goto next_item; + /* + * Locks on ON COMMIT DELETE ROWS temp tables are released in + * PREPARE TRANSACTION phase + */ + if (lock->tag.locktag_type == LOCKTAG_RELATION) + { + Oid relid = lock->tag.locktag_field2; + if (check_prepare_safe_temp_table(relid)) + goto next_item; + } + PROCLOCK_PRINT("PostPrepare_Locks", proclock); LOCK_PRINT("PostPrepare_Locks", lock, 0); Assert(lock->nRequested >= 0); diff --git a/src/include/access/xact.h b/src/include/access/xact.h index dad6ef8..5fca0d6 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -44,9 +44,6 @@ 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 */ diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 5a43d27..81dd51c 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -63,6 +63,7 @@ extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno); extern void register_on_commit_action(Oid relid, OnCommitAction action); extern void remove_on_commit_action(Oid relid); +extern bool check_prepare_safe_temp_table(Oid relid); extern void PreCommit_on_commit_actions(void); extern void AtEOXact_on_commit_actions(bool isCommit);
Heikki Linnakangas wrote: > Somehow this feels pretty baroque, though. Perhaps a better approach > would be to add a new AtPrepare_OnCommitActions function to tablecmds.c, > that gets called before AtPrepare_Locks. It would scan through the > on_commits list, and release all locks for the "PREPARE-safe" temp > tables, and throw the error if necessary. I'll try that next. Here's what I ended up with. I morphed the on commit action registration into tracking of all temporary relations. This only allows access to ON COMMIT DELETE ROWS temp tables. Accessing other temporary tables, and creating or dropping tables in the transaction is still forbidden. It took me a couple of iterations to handle toast tables and indexes correctly. More testing would be appreciated with more complex cases like VACUUM FULL, subtransactions etc. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com *** src/backend/access/heap/heapam.c --- src/backend/access/heap/heapam.c *************** *** 51,56 **** --- 51,57 ---- #include "access/xlogutils.h" #include "catalog/catalog.h" #include "catalog/namespace.h" + #include "commands/tablecmds.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/bufmgr.h" *************** *** 878,884 **** relation_open(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) ! MyXactAccessedTempRel = true; pgstat_initstats(r); --- 879,885 ---- /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) ! register_temp_rel_access(relationId); pgstat_initstats(r); *************** *** 926,932 **** try_relation_open(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) ! MyXactAccessedTempRel = true; pgstat_initstats(r); --- 927,933 ---- /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) ! register_temp_rel_access(relationId); pgstat_initstats(r); *************** *** 976,982 **** relation_open_nowait(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) ! MyXactAccessedTempRel = true; pgstat_initstats(r); --- 977,983 ---- /* Make note that we've accessed a temporary relation */ if (r->rd_istemp) ! register_temp_rel_access(relationId); pgstat_initstats(r); *** src/backend/access/transam/xact.c --- src/backend/access/transam/xact.c *************** *** 67,80 **** 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 */ typedef enum TransState --- 67,72 ---- *************** *** 1467,1473 **** StartTransaction(void) XactIsoLevel = DefaultXactIsoLevel; XactReadOnly = DefaultXactReadOnly; forceSyncCommit = false; - MyXactAccessedTempRel = false; /* * reinitialize within-transaction counters --- 1459,1464 ---- *************** *** 1798,1823 **** 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(); --- 1789,1794 ---- *************** *** 1861,1866 **** PrepareTransaction(void) --- 1832,1838 ---- AtPrepare_Notify(); AtPrepare_UpdateFlatFiles(); AtPrepare_Inval(); + AtPrepare_on_commit_actions(); AtPrepare_Locks(); AtPrepare_PgStat(); *** src/backend/catalog/heap.c --- src/backend/catalog/heap.c *************** *** 39,44 **** --- 39,45 ---- #include "catalog/heap.h" #include "catalog/index.h" #include "catalog/indexing.h" + #include "catalog/namespace.h" #include "catalog/pg_attrdef.h" #include "catalog/pg_constraint.h" #include "catalog/pg_inherits.h" *************** *** 1062,1071 **** heap_create_with_catalog(const char *relname, StoreConstraints(new_rel_desc, cooked_constraints); /* ! * If there's a special on-commit action, remember it */ ! if (oncommit != ONCOMMIT_NOOP) ! register_on_commit_action(relid, oncommit); /* * ok, the relation has been cataloged, so close our relations and return --- 1063,1072 ---- StoreConstraints(new_rel_desc, cooked_constraints); /* ! * Register to the on commit action handler, if it's a temporary table. */ ! if (isTempOrToastNamespace(relnamespace)) ! register_temp_rel(relid, relkind, oncommit); /* * ok, the relation has been cataloged, so close our relations and return *************** *** 1443,1451 **** heap_drop_with_catalog(Oid relid) relation_close(rel, NoLock); /* ! * Forget any ON COMMIT action for the rel */ ! remove_on_commit_action(relid); /* * Flush the relation from the relcache. We want to do this before --- 1444,1452 ---- relation_close(rel, NoLock); /* ! * Unregister any ON COMMIT action for the relation. */ ! unregister_temp_rel(relid); /* * Flush the relation from the relcache. We want to do this before *** src/backend/catalog/index.c --- src/backend/catalog/index.c *************** *** 873,878 **** index_create(Oid heapRelationId, --- 873,886 ---- } /* + * Register to the on commit action handler, if it's a temporary table. + * Indexes can't have any ON COMMIT actions, but the registration is + * still needed to handle PREPARE TRANSACTION correctly. + */ + if (isTempOrToastNamespace(namespaceId)) + register_temp_rel(indexRelationId, RELKIND_INDEX, ONCOMMIT_NOOP); + + /* * Close the heap and index; but we keep the locks that we acquired above * until end of transaction. */ *************** *** 953,958 **** index_drop(Oid indexId) --- 961,971 ---- heap_close(indexRelation, RowExclusiveLock); /* + * Unregister from the on commit action handler. + */ + unregister_temp_rel(indexId); + + /* * if it has any expression columns, we might have stored statistics about * them. */ *** src/backend/commands/tablecmds.c --- src/backend/commands/tablecmds.c *************** *** 83,88 **** --- 83,89 ---- typedef struct OnCommitItem { Oid relid; /* relid of relation */ + char relkind; /* relkind of relation */ OnCommitAction oncommit; /* what to do at end of xact */ /* *************** *** 94,102 **** typedef struct OnCommitItem */ SubTransactionId creating_subid; SubTransactionId deleting_subid; } OnCommitItem; ! static List *on_commits = NIL; /* --- 95,104 ---- */ SubTransactionId creating_subid; SubTransactionId deleting_subid; + SubTransactionId accessing_subid; } OnCommitItem; ! static HTAB *on_commits = NULL; /* *************** *** 7561,7600 **** AlterSeqNamespaces(Relation classRel, Relation rel, /* ! * This code supports * CREATE TEMP TABLE ... ON COMMIT { DROP | PRESERVE ROWS | DELETE ROWS } * * Because we only support this for TEMP tables, it's sufficient to remember * the state in a backend-local data structure. */ /* ! * Register a newly-created relation's ON COMMIT action. */ void ! register_on_commit_action(Oid relid, OnCommitAction action) { OnCommitItem *oc; ! MemoryContext oldcxt; ! /* ! * We needn't bother registering the relation unless there is an ON COMMIT ! * action we need to take. ! */ ! if (action == ONCOMMIT_NOOP || action == ONCOMMIT_PRESERVE_ROWS) ! return; ! oldcxt = MemoryContextSwitchTo(CacheMemoryContext); - oc = (OnCommitItem *) palloc(sizeof(OnCommitItem)); oc->relid = relid; oc->oncommit = action; oc->creating_subid = GetCurrentSubTransactionId(); oc->deleting_subid = InvalidSubTransactionId; ! ! on_commits = lcons(oc, on_commits); ! ! MemoryContextSwitchTo(oldcxt); } /* --- 7563,7615 ---- /* ! * This code keeps track of all temporary relations in the current session, ! * to support * CREATE TEMP TABLE ... ON COMMIT { DROP | PRESERVE ROWS | DELETE ROWS } * + * and to implement the special handling of temporary relations at + * PREPARE TRANSACTION. + * * Because we only support this for TEMP tables, it's sufficient to remember * the state in a backend-local data structure. */ /* ! * Register a newly-created temporary relation. */ void ! register_temp_rel(Oid relid, char relkind, OnCommitAction action) { OnCommitItem *oc; ! bool found; ! if (on_commits == NULL) ! { ! HASHCTL hash_ctl; ! ! /* Initialize hash table */ ! memset(&hash_ctl, 0, sizeof(hash_ctl)); ! hash_ctl.keysize = sizeof(Oid); ! hash_ctl.entrysize = sizeof(OnCommitItem); ! hash_ctl.hcxt = CacheMemoryContext; ! hash_ctl.hash = oid_hash; ! ! on_commits = hash_create("On commit actions", ! 4, /* small initial size to make scans fast */ ! &hash_ctl, ! HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT); ! } ! oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_ENTER, &found); ! if (found) ! elog(ERROR, "relid already in on commit action table"); oc->relid = relid; + oc->relkind = relkind; oc->oncommit = action; oc->creating_subid = GetCurrentSubTransactionId(); oc->deleting_subid = InvalidSubTransactionId; ! oc->accessing_subid = InvalidSubTransactionId; } /* *************** *** 7603,7622 **** register_on_commit_action(Oid relid, OnCommitAction action) * Actually, we only mark the OnCommitItem entry as to be deleted after commit. */ void ! remove_on_commit_action(Oid relid) { ! ListCell *l; ! foreach(l, on_commits) ! { ! OnCommitItem *oc = (OnCommitItem *) lfirst(l); ! if (oc->relid == relid) ! { ! oc->deleting_subid = GetCurrentSubTransactionId(); ! break; ! } ! } } /* --- 7618,7650 ---- * Actually, we only mark the OnCommitItem entry as to be deleted after commit. */ void ! unregister_temp_rel(Oid relid) { ! OnCommitItem *oc; ! if (on_commits == NULL) ! return; ! oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_FIND, NULL); ! ! if (oc != NULL) ! oc->deleting_subid = GetCurrentSubTransactionId(); ! } ! ! ! /* ! * Make note that a temporary relation has been accessed in this transaction. ! * This should be called whenever a lock is acquired on the relation, except ! * for transient locks that are always released before commit. ! * relation_open() will usually do that for you. ! */ ! void ! register_temp_rel_access(Oid relid) ! { ! OnCommitItem *oc; ! ! oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_FIND, NULL); ! oc->accessing_subid = GetCurrentSubTransactionId(); } /* *************** *** 7628,7672 **** remove_on_commit_action(Oid relid) void PreCommit_on_commit_actions(void) { ! ListCell *l; List *oids_to_truncate = NIL; ! foreach(l, on_commits) ! { ! OnCommitItem *oc = (OnCommitItem *) lfirst(l); ! ! /* Ignore entry if already dropped in this xact */ ! if (oc->deleting_subid != InvalidSubTransactionId) ! continue; ! switch (oc->oncommit) { ! case ONCOMMIT_NOOP: ! case ONCOMMIT_PRESERVE_ROWS: ! /* Do nothing (there shouldn't be such entries, actually) */ ! break; ! case ONCOMMIT_DELETE_ROWS: ! oids_to_truncate = lappend_oid(oids_to_truncate, oc->relid); ! break; ! case ONCOMMIT_DROP: ! { ! ObjectAddress object; ! ! object.classId = RelationRelationId; ! object.objectId = oc->relid; ! object.objectSubId = 0; ! performDeletion(&object, DROP_CASCADE); ! /* ! * Note that table deletion will call ! * remove_on_commit_action, so the entry should get marked ! * as deleted. ! */ ! Assert(oc->deleting_subid != InvalidSubTransactionId); break; ! } } } if (oids_to_truncate != NIL) { heap_truncate(oids_to_truncate); --- 7656,7712 ---- void PreCommit_on_commit_actions(void) { ! HASH_SEQ_STATUS seq_status; List *oids_to_truncate = NIL; + OnCommitItem *oc; ! if (on_commits == NULL) ! return; ! hash_seq_init(&seq_status, on_commits); ! PG_TRY(); ! { ! while ((oc = hash_seq_search(&seq_status)) != NULL) { ! /* Ignore entry if already dropped in this xact */ ! if (oc->deleting_subid != InvalidSubTransactionId) ! continue; ! switch (oc->oncommit) ! { ! case ONCOMMIT_NOOP: ! case ONCOMMIT_PRESERVE_ROWS: ! /* Do nothing */ break; ! case ONCOMMIT_DELETE_ROWS: ! oids_to_truncate = lappend_oid(oids_to_truncate, oc->relid); ! break; ! case ONCOMMIT_DROP: ! { ! ObjectAddress object; ! ! object.classId = RelationRelationId; ! object.objectId = oc->relid; ! object.objectSubId = 0; ! performDeletion(&object, DROP_CASCADE); ! ! /* ! * Note that table deletion will call ! * remove_on_commit_action, so the entry should get ! * marked as deleted. ! */ ! Assert(oc->deleting_subid != InvalidSubTransactionId); ! break; ! } ! } } } + PG_CATCH(); + { + hash_seq_term(&seq_status); + } + PG_END_TRY(); + if (oids_to_truncate != NIL) { heap_truncate(oids_to_truncate); *************** *** 7675,7680 **** PreCommit_on_commit_actions(void) --- 7715,7821 ---- } /* + * 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. + * + * However, since PostgreSQL 8.4, we do allow PREPARE TRANSATION if the + * transaction has accessed a temporary table with ON COMMIT DELETE ROWS + * action, as long as the transaction hasn't created or dropped one. We + * work around the lock problem by releasing the locks early, in the PREPARE + * phase. That doesn't violate the two-phase locking protocol (not to be + * confused with two-phase commit), as the lock would be released at the + * 2nd phase commit or rollback anyway, and the transaction won't acquire + * any more locks after PREPARE. + * + * The reason why we only allow that for ON COMMIT DELETE ROWS tables, is that + * some commands like VACUUM FULL and CREATE INDEX get confused if there's + * xids in the table that belong to an in-progress transaction. They don't + * normally encounter such tuples because they grab an AccessExclusiveLock. + * + * XXX In principle this could be relaxed to allow some other useful special + * cases, such as a temp table created and dropped all within the + * transaction. We would need to at least drop the local buffers, however. + * It might also be possible to allow read-only access to temp tables, they + * shouldn't pose a problem to commands like VACUUM FULL and CREATE INDEX. + */ + void + AtPrepare_on_commit_actions(void) + { + HASH_SEQ_STATUS seq_status; + OnCommitItem *oc; + + if (on_commits == NULL) + return; + + hash_seq_init(&seq_status, on_commits); + while ((oc = hash_seq_search(&seq_status)) != NULL) + { + /* + * If the transaction hasn't touched the relation at all, we're fine. + */ + if (oc->accessing_subid == InvalidSubTransactionId && + oc->creating_subid == InvalidSubTransactionId && + oc->deleting_subid == InvalidSubTransactionId) + continue; + + /* + * Allow access to ON COMMIT DELETE ROWS temporary tables, + * that have not been created or dropped in this transaction. + */ + if ((oc->oncommit == ONCOMMIT_DELETE_ROWS || oc->relkind != RELKIND_RELATION) && + oc->creating_subid == InvalidSubTransactionId && + oc->deleting_subid == InvalidSubTransactionId) + { + /* + * XXX If we could release the locks on the relation here, + * AtPrepare_Locks() wouldn't need to treat temp locks specially, + * and we wouldn't need the is_temp_rel() function below. But we + * don't know what locks we're holding, and there's no function + * to release all our locks on a given object. + */ + continue; + } + else + { + hash_seq_term(&seq_status); + /* Determine appropriate error message */ + if (oc->creating_subid != InvalidSubTransactionId || + oc->deleting_subid != InvalidSubTransactionId) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot PREPARE a transaction that has created or dropped a temporary relation"))); + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot PREPARE a transaction that has accessed a temporary table not defined with ON COMMITDELETE ROWS"))); + } + } + } + + /* + * Is the given relation a temporary table in this backend? + */ + bool + is_temp_rel(Oid relid) + { + OnCommitItem *oc; + + /* you can't access a temp table if there isn't any */ + Assert(on_commits != NULL); + + oc = (OnCommitItem *) hash_search(on_commits, &relid, HASH_FIND, NULL); + + return (oc != NULL); + } + + /* * Post-commit or post-abort cleanup for ON COMMIT management. * * All we do here is remove no-longer-needed OnCommitItem entries. *************** *** 7685,7718 **** PreCommit_on_commit_actions(void) void AtEOXact_on_commit_actions(bool isCommit) { ! ListCell *cur_item; ! ListCell *prev_item; ! prev_item = NULL; ! cur_item = list_head(on_commits); ! while (cur_item != NULL) { - OnCommitItem *oc = (OnCommitItem *) lfirst(cur_item); - if (isCommit ? oc->deleting_subid != InvalidSubTransactionId : oc->creating_subid != InvalidSubTransactionId) { ! /* cur_item must be removed */ ! on_commits = list_delete_cell(on_commits, cur_item, prev_item); ! pfree(oc); ! if (prev_item) ! cur_item = lnext(prev_item); ! else ! cur_item = list_head(on_commits); } else { ! /* cur_item must be preserved */ oc->creating_subid = InvalidSubTransactionId; oc->deleting_subid = InvalidSubTransactionId; ! prev_item = cur_item; ! cur_item = lnext(prev_item); } } } --- 7826,7852 ---- void AtEOXact_on_commit_actions(bool isCommit) { ! HASH_SEQ_STATUS seq_status; ! OnCommitItem *oc; ! if (on_commits == NULL) ! return; ! hash_seq_init(&seq_status, on_commits); ! while ((oc = hash_seq_search(&seq_status)) != NULL) { if (isCommit ? oc->deleting_subid != InvalidSubTransactionId : oc->creating_subid != InvalidSubTransactionId) { ! /* current item must be removed */ ! hash_search(on_commits, &oc->relid, HASH_REMOVE, NULL); } else { ! /* current item must be preserved */ oc->creating_subid = InvalidSubTransactionId; oc->deleting_subid = InvalidSubTransactionId; ! oc->accessing_subid = InvalidSubTransactionId; } } } *************** *** 7728,7762 **** void AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid, SubTransactionId parentSubid) { ! ListCell *cur_item; ! ListCell *prev_item; ! prev_item = NULL; ! cur_item = list_head(on_commits); ! while (cur_item != NULL) { - OnCommitItem *oc = (OnCommitItem *) lfirst(cur_item); - if (!isCommit && oc->creating_subid == mySubid) { ! /* cur_item must be removed */ ! on_commits = list_delete_cell(on_commits, cur_item, prev_item); ! pfree(oc); ! if (prev_item) ! cur_item = lnext(prev_item); ! else ! cur_item = list_head(on_commits); } else { ! /* cur_item must be preserved */ if (oc->creating_subid == mySubid) oc->creating_subid = parentSubid; if (oc->deleting_subid == mySubid) oc->deleting_subid = isCommit ? parentSubid : InvalidSubTransactionId; ! prev_item = cur_item; ! cur_item = lnext(prev_item); } } } --- 7862,7890 ---- AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid, SubTransactionId parentSubid) { ! HASH_SEQ_STATUS seq_status; ! OnCommitItem *oc; ! if (on_commits == NULL) ! return; ! hash_seq_init(&seq_status, on_commits); ! while ((oc = hash_seq_search(&seq_status)) != NULL) { if (!isCommit && oc->creating_subid == mySubid) { ! /* current item must be removed */ ! hash_search(on_commits, &oc->relid, HASH_REMOVE, NULL); } else { ! /* current item must be preserved */ if (oc->creating_subid == mySubid) oc->creating_subid = parentSubid; if (oc->deleting_subid == mySubid) oc->deleting_subid = isCommit ? parentSubid : InvalidSubTransactionId; ! if (oc->accessing_subid == mySubid) ! oc->accessing_subid = isCommit ? parentSubid : InvalidSubTransactionId; } } } *** src/backend/storage/lmgr/lock.c --- src/backend/storage/lmgr/lock.c *************** *** 35,40 **** --- 35,41 ---- #include "access/transam.h" #include "access/twophase.h" #include "access/twophase_rmgr.h" + #include "commands/tablecmds.h" #include "miscadmin.h" #include "pg_trace.h" #include "pgstat.h" *************** *** 1816,1834 **** GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode) return vxids; } /* * AtPrepare_Locks * Do the preparatory work for a PREPARE: make 2PC state file records * for all locks currently held. * ! * Non-transactional locks are ignored, as are VXID locks. * * There are some special cases that we error out on: we can't be holding ! * any session locks (should be OK since only VACUUM uses those) and we ! * can't be holding any locks on temporary objects (since that would mess ! * up the current backend if it tries to exit before the prepared xact is ! * committed). */ void AtPrepare_Locks(void) --- 1817,1882 ---- return vxids; } + /* + * A helper function to determine whether the given lock should be + * prepared + */ + static bool + is_preparable_locktag(LOCKTAG *lock) + { + /* Ignore nontransactional locks */ + if (!LockMethods[lock->locktag_lockmethodid]->transactional) + return false; + + /* Locks on temporary tables are dropped in PREPARE phase */ + switch (lock->locktag_type) + { + case LOCKTAG_RELATION_EXTEND: + case LOCKTAG_PAGE: + case LOCKTAG_TUPLE: + /* + * These lock types are only held transiently, we shouldn't + * be holding any at the end of transaction. + */ + elog(ERROR, "unexpected lock type at PREPARE TRANSACTION"); + + case LOCKTAG_VIRTUALTRANSACTION: + /* + * Ignore VXID locks. We don't want those to be held by + * prepared transactions, since they aren't meaningful after + * a restart. + */ + return false; + + case LOCKTAG_RELATION: + /* + * Locks on temporary relations are not persisted. That means + * that they're dropped early, in the PREPARE phase. We only + * allow that in some narrow cases, which + * AtPrepare_on_commit_actions has already checked for. + */ + /* field1 is dboid, field2 is reloid for all of these */ + if ((Oid) lock->locktag_field1 == InvalidOid) + break; /* shared, so not temp */ + if (is_temp_rel((Oid) lock->locktag_field2)) + return false; + default: + break; + } + return true; + } /* * AtPrepare_Locks * Do the preparatory work for a PREPARE: make 2PC state file records * for all locks currently held. * ! * Non-transactional and VXID locks are ignored. Locks on temporary objects ! * are also ignored, because that would mess up the current backend if it ! * tries to exit before the prepared xact is committed. * * There are some special cases that we error out on: we can't be holding ! * any session locks (should be OK since only VACUUM uses those). */ void AtPrepare_Locks(void) *************** *** 1848,1868 **** AtPrepare_Locks(void) LOCALLOCKOWNER *lockOwners = locallock->lockOwners; int i; - /* Ignore nontransactional locks */ - if (!LockMethods[LOCALLOCK_LOCKMETHOD(*locallock)]->transactional) - continue; - - /* - * Ignore VXID locks. We don't want those to be held by prepared - * transactions, since they aren't meaningful after a restart. - */ - if (locallock->tag.lock.locktag_type == LOCKTAG_VIRTUALTRANSACTION) - continue; - /* Ignore it if we don't actually hold the lock */ if (locallock->nLocks <= 0) continue; /* Scan to verify there are no session locks */ for (i = locallock->numLockOwners - 1; i >= 0; i--) { --- 1896,1909 ---- LOCALLOCKOWNER *lockOwners = locallock->lockOwners; int i; /* Ignore it if we don't actually hold the lock */ if (locallock->nLocks <= 0) continue; + /* Does the lock need to be persisted? */ + if (!is_preparable_locktag(&locallock->tag.lock)) + continue; + /* Scan to verify there are no session locks */ for (i = locallock->numLockOwners - 1; i >= 0; i--) { *************** *** 1936,1947 **** PostPrepare_Locks(TransactionId xid) continue; } ! /* Ignore nontransactional locks */ ! if (!LockMethods[LOCALLOCK_LOCKMETHOD(*locallock)]->transactional) ! continue; ! ! /* Ignore VXID locks */ ! if (locallock->tag.lock.locktag_type == LOCKTAG_VIRTUALTRANSACTION) continue; /* We already checked there are no session locks */ --- 1977,1984 ---- continue; } ! /* Does the lock need to be persisted? */ ! if (!is_preparable_locktag(&locallock->tag.lock)) continue; /* We already checked there are no session locks */ *************** *** 1985,1996 **** PostPrepare_Locks(TransactionId xid) lock = proclock->tag.myLock; ! /* Ignore nontransactional locks */ ! if (!LockMethods[LOCK_LOCKMETHOD(*lock)]->transactional) ! goto next_item; ! ! /* Ignore VXID locks */ ! if (lock->tag.locktag_type == LOCKTAG_VIRTUALTRANSACTION) goto next_item; PROCLOCK_PRINT("PostPrepare_Locks", proclock); --- 2022,2029 ---- lock = proclock->tag.myLock; ! /* Does the lock need to be persisted? */ ! if (!is_preparable_locktag(&lock->tag)) goto next_item; PROCLOCK_PRINT("PostPrepare_Locks", proclock); *** src/include/access/xact.h --- src/include/access/xact.h *************** *** 44,52 **** 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 */ --- 44,49 ---- *** src/include/commands/tablecmds.h --- src/include/commands/tablecmds.h *************** *** 61,69 **** extern AttrNumber *varattnos_map(TupleDesc old, TupleDesc new); extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema); extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno); ! extern void register_on_commit_action(Oid relid, OnCommitAction action); ! extern void remove_on_commit_action(Oid relid); extern void PreCommit_on_commit_actions(void); extern void AtEOXact_on_commit_actions(bool isCommit); extern void AtEOSubXact_on_commit_actions(bool isCommit, --- 61,72 ---- extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema); extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno); ! extern void register_temp_rel(Oid relid, char relkind, OnCommitAction action); ! extern void unregister_temp_rel(Oid relid); ! extern void register_temp_rel_access(Oid relid); ! extern bool is_temp_rel(Oid relid); + extern void AtPrepare_on_commit_actions(void); extern void PreCommit_on_commit_actions(void); extern void AtEOXact_on_commit_actions(bool isCommit); extern void AtEOSubXact_on_commit_actions(bool isCommit,
Hi Heikki, I will try to make more tests. I still quite did not get what the big deal was if an ON COMMIT DELETE ROWS temp table was created inside a transaction. Why the new checks you are doing in lock.c would not work with dropped temp tables? Could it be possible to drop the lock as soon as the temp table is dropped inside a transaction? I will try to find more time to review the patch tonight. Best, Emmanuel > Heikki Linnakangas wrote: >> Somehow this feels pretty baroque, though. Perhaps a better approach >> would be to add a new AtPrepare_OnCommitActions function to >> tablecmds.c, that gets called before AtPrepare_Locks. It would scan >> through the on_commits list, and release all locks for the >> "PREPARE-safe" temp tables, and throw the error if necessary. I'll >> try that next. > > Here's what I ended up with. I morphed the on commit action > registration into tracking of all temporary relations. > > This only allows access to ON COMMIT DELETE ROWS temp tables. > Accessing other temporary tables, and creating or dropping tables in > the transaction is still forbidden. > > It took me a couple of iterations to handle toast tables and indexes > correctly. More testing would be appreciated with more complex cases > like VACUUM FULL, subtransactions etc. > -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Emmanuel Cecchet wrote: > I still quite did not get what the big deal was if an ON COMMIT DELETE > ROWS temp table was created inside a transaction. In case the transaction that created a temp table rolls back, the table needs to be removed. Removing a temporary table belonging to another backend is problematic; the local buffers in the original backend need to be dropped, as well as the entry in the on commit actions list. > Why the new checks you are doing in lock.c would not work with dropped > temp tables? Could it be possible to drop the lock as soon as the temp > table is dropped inside a transaction? If you release the lock early on a table that you drop, another transactions would be free to access the table, even though it's about to be dropped. > I will try to find more time to review the patch tonight. Thanks! Thinking about this whole thing yet more, I wonder if we could have a more holistic approach and make temporary tables work just like regular ones. The problems we've identified this far are: 1. If the prepared transaction keeps the temp table locked, the backend can't exit, because the shutdown hook tries to drop all temp tables. 2. When a prepared transaction that has deleted a temporary table commits (or one that created one aborts), we need to drop all the local buffers from the original backend's private buffer cache. 3. When a prepared transaction that has deleted a temporary table commits (or one that created one aborts), we need to remove the on-commit entry from the original backend's private list. Is there more? I think we can solve all the above problems: 1. Modify RemoveTempRelations so that it doesn't block if it can't immediately acquire lock on the to-be-removed object. That way the original backend can exit even if a prepared transaction is holding a lock on a temporary object. To avoid conflict with a new backend that's assigned the same backendid, divorce the temporary namespace naming from backendid so that the temporary namespace name stays reserved for the prepared transaction. 2. Flush and drop all local buffers on temporary tables that have been created or dropped in the transaction at PREPARE TRANSACTION already. 3. Add on-commit field to pg_class, and only keep a list of temporary tables that have been accessed in the current transaction in backend-private memory. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: > Emmanuel Cecchet wrote: >> I still quite did not get what the big deal was if an ON COMMIT >> DELETE ROWS temp table was created inside a transaction. > > In case the transaction that created a temp table rolls back, the > table needs to be removed. Removing a temporary table belonging to > another backend is problematic; the local buffers in the original > backend need to be dropped, as well as the entry in the on commit > actions list. > >> Why the new checks you are doing in lock.c would not work with >> dropped temp tables? Could it be possible to drop the lock as soon as >> the temp table is dropped inside a transaction? > > If you release the lock early on a table that you drop, another > transactions would be free to access the table, even though it's about > to be dropped. > >> I will try to find more time to review the patch tonight. > > Thanks! > > Thinking about this whole thing yet more, I wonder if we could have a > more holistic approach and make temporary tables work just like > regular ones. The problems we've identified this far are: > > 1. If the prepared transaction keeps the temp table locked, the > backend can't exit, because the shutdown hook tries to drop all temp > tables. > > 2. When a prepared transaction that has deleted a temporary table > commits (or one that created one aborts), we need to drop all the > local buffers from the original backend's private buffer cache. > > 3. When a prepared transaction that has deleted a temporary table > commits (or one that created one aborts), we need to remove the > on-commit entry from the original backend's private list. > > Is there more? I think we can solve all the above problems: > > 1. Modify RemoveTempRelations so that it doesn't block if it can't > immediately acquire lock on the to-be-removed object. That way the > original backend can exit even if a prepared transaction is holding a > lock on a temporary object. > > To avoid conflict with a new backend that's assigned the same > backendid, divorce the temporary namespace naming from backendid so > that the temporary namespace name stays reserved for the prepared > transaction. Is that going to cause any problem with DROP CASCADE operations or trying to later drop a child table if the parent table is locked? I did hit that issue when I tried to modify RemoveTempRelations but I was probably not very smart at it. > 2. Flush and drop all local buffers on temporary tables that have been > created or dropped in the transaction at PREPARE TRANSACTION already. Would there be any issue if someone was trying to use a READ_UNCOMMITTED isolation level to access the temp table data? > 3. Add on-commit field to pg_class, and only keep a list of temporary > tables that have been accessed in the current transaction in > backend-private memory. Yes, this seems doable. We will certainly have to keep a list per transaction id in case multiple prepared but uncommitted transactions have accessed different temp table on that backend. Have you already started to code some of this? I am looking at adding new tests to check all cases including all types of temp tables (normal/on delete/on drop, created inside or outside a transaction, prepare/postmaster stop/commit/rollback, inherit/index/vaccuum). That should get all the use cases covered. Let me know what you think. Thanks, Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Heikki,
The following test fails with your patch on my system. Could you check
if you can reproduce?
psql (8.4devel)
Type "help" for help.
test=# begin;
BEGIN
test=# create table paul(x int);
CREATE TABLE
test=# insert into paul values(1);
INSERT 0 1
test=# prepare transaction 'persistentTableShouldSucceed';
server closed the connection unexpectedly This probably means the server terminated abnormally before or
whileprocessing the request.
The connection to the server was lost. Attempting reset: Failed.
---
LOG: database system is ready to accept connections
TRAP: FailedAssertion("!(on_commits != ((void *)0))", File:
"tablecmds.c", Line: 7823)
LOG: server process (PID 15969) was terminated by signal 6: Aborted
LOG: terminating any other active server processes
FATAL: the database system is in recovery mode
Thanks,
manu
Heikki,
I think that the Assert in is_temp_rel(Oid) in tablecmds.c should be
replaced by if (on_commits == NULL) return false;
As the use case below shows, a regular table can be created and hold a
LOCKTAG_RELATION lock that will trigger the call to is_temp_rel in
is_preparable_locktag. The assert will break if no temp table was accessed.
As we were also trying to list potential issues, if the temp table uses
a SERIAL type, will there be potentially a problem with the sequence at
prepare time?
Emmanuel
> The following test fails with your patch on my system. Could you check
> if you can reproduce?
>
> psql (8.4devel)
> Type "help" for help.
>
> test=# begin;
> BEGIN
> test=# create table paul(x int);
> CREATE TABLE
> test=# insert into paul values(1);
> INSERT 0 1
> test=# prepare transaction 'persistentTableShouldSucceed';
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
>
> ---
>
> LOG: database system is ready to accept connections
> TRAP: FailedAssertion("!(on_commits != ((void *)0))", File:
> "tablecmds.c", Line: 7823)
> LOG: server process (PID 15969) was terminated by signal 6: Aborted
> LOG: terminating any other active server processes
> FATAL: the database system is in recovery mode
>
>
> Thanks,
> manu
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
Heikki,
I have extended the patch to allow temp tables that have been
created/dropped within the same transaction (and also on commit drop).
There is a problem with temp tables with on delete rows that are created
inside a transaction.
Take the 2pc_on_delete_rows_transaction.sql test case and change the
creation statement, instead of
create temp table foo(x int) on commit delete rows;
try
create temp table foo(x serial primary key) on commit delete rows;
The test will fail. It looks like the onCommit field is not properly
updated when serial or primary key is used in that context. I did not
figure out why.
Waiting for your feedback
Emmanuel
Emmanuel Cecchet wrote:
> I think that the Assert in is_temp_rel(Oid) in tablecmds.c should be
> replaced by if (on_commits == NULL) return false;
> As the use case below shows, a regular table can be created and hold a
> LOCKTAG_RELATION lock that will trigger the call to is_temp_rel in
> is_preparable_locktag. The assert will break if no temp table was
> accessed.
>
> As we were also trying to list potential issues, if the temp table
> uses a SERIAL type, will there be potentially a problem with the
> sequence at prepare time?
>
> Emmanuel
>
>
>> The following test fails with your patch on my system. Could you
>> check if you can reproduce?
>>
>> psql (8.4devel)
>> Type "help" for help.
>>
>> test=# begin;
>> BEGIN
>> test=# create table paul(x int);
>> CREATE TABLE
>> test=# insert into paul values(1);
>> INSERT 0 1
>> test=# prepare transaction 'persistentTableShouldSucceed';
>> server closed the connection unexpectedly
>> This probably means the server terminated abnormally
>> before or while processing the request.
>> The connection to the server was lost. Attempting reset: Failed.
>>
>> ---
>>
>> LOG: database system is ready to accept connections
>> TRAP: FailedAssertion("!(on_commits != ((void *)0))", File:
>> "tablecmds.c", Line: 7823)
>> LOG: server process (PID 15969) was terminated by signal 6: Aborted
>> LOG: terminating any other active server processes
>> FATAL: the database system is in recovery mode
>>
>>
>> Thanks,
>> manu
>>
>
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
Attachment
Emmanuel Cecchet wrote: > I think that the Assert in is_temp_rel(Oid) in tablecmds.c should be > replaced by if (on_commits == NULL) return false; > As the use case below shows, a regular table can be created and hold a > LOCKTAG_RELATION lock that will trigger the call to is_temp_rel in > is_preparable_locktag. The assert will break if no temp table was accessed. Yes, you're right. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Emmanuel Cecchet wrote: > There is a problem with temp tables with on delete rows that are created > inside a transaction. > Take the 2pc_on_delete_rows_transaction.sql test case and change the > creation statement, instead of > create temp table foo(x int) on commit delete rows; > try > create temp table foo(x serial primary key) on commit delete rows; > > The test will fail. It looks like the onCommit field is not properly > updated when serial or primary key is used in that context. I did not > figure out why. A serial column uses a sequence behind the scenes. Hmm. Seems like we would need to treat sequences and indexes the same as tables with ON COMMIT DELETE ROWS, i.e release the locks early and don't error out. All in all, this is getting pretty messy. My patch felt a bit hackish to begin with, and having to add special cases for sequences and indexes would make it even more so. And what about temporary views? I'm starting to feel that instead of special-casing temp relations, we need to move into the opposite direction and make temp relations more like regular relations. Unfortunately, that's not going to happen in the 8.4 timeframe :-(. Let's try the other approach in 8.5. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
I would really like to have support for temp tables at least for the case where the table is created and dropped in the same transaction. But I guess that the other limitations on index, sequences and views would still hold, right? manu Heikki Linnakangas wrote: > Emmanuel Cecchet wrote: >> There is a problem with temp tables with on delete rows that are >> created inside a transaction. >> Take the 2pc_on_delete_rows_transaction.sql test case and change the >> creation statement, instead of >> create temp table foo(x int) on commit delete rows; >> try >> create temp table foo(x serial primary key) on commit delete rows; >> >> The test will fail. It looks like the onCommit field is not properly >> updated when serial or primary key is used in that context. I did not >> figure out why. > A serial column uses a sequence behind the scenes. > > Hmm. Seems like we would need to treat sequences and indexes the same > as tables with ON COMMIT DELETE ROWS, i.e release the locks early and > don't error out. > > All in all, this is getting pretty messy. My patch felt a bit hackish > to begin with, and having to add special cases for sequences and > indexes would make it even more so. And what about temporary views? > I'm starting to feel that instead of special-casing temp relations, we > need to move into the opposite direction and make temp relations more > like regular relations. Unfortunately, that's not going to happen in > the 8.4 timeframe :-(. Let's try the other approach in 8.5. > -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet
Hi all, Here is a new version of the patch still based on the last proposal by Heikki. This patch only support temp tables that are created AND dropped in the same transaction. This works with any kind of temp table (with on commit options or not). This simplifies the problem since the temp tables we consider here cannot be accessed by another backend anyway since their scope is the one of the transaction. We do NOT allow: - access to temp tables that were not created AND dropped in the transaction - created temp tables that are not dropped at prepare time - dropped temp tables that were not created in the transaction The attached patch has a number of tests for all these cases including sequences and index on temp tables. Let me know what you think Emmanuel -- Emmanuel Cecchet FTO @ Frog Thinker Open Source Development & Consulting -- Web: http://www.frogthinker.org email: manu@frogthinker.org Skype: emmanuel_cecchet