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 COMMIT DELETE 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);