diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index b1852779ccfbc039d5b609ece2ac64ae33b6eeb2..60b5d3bd514b5e6ff610e23fd6e47b2a84a584e5 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -6,7 +6,7 @@ * Copyright (c) 2000-2010, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/varsup.c,v 1.89 2010/02/17 03:10:33 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/varsup.c,v 1.90 2010/02/20 21:24:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -58,6 +58,10 @@ GetNewTransactionId(bool isSubXact) return BootstrapTransactionId; } + /* safety check, we should never get this far in a HS slave */ + if (RecoveryInProgress()) + elog(ERROR, "cannot assign TransactionIds during recovery"); + LWLockAcquire(XidGenLock, LW_EXCLUSIVE); xid = ShmemVariableCache->nextXid; @@ -420,6 +424,10 @@ GetNewObjectId(void) { Oid result; + /* safety check, we should never get this far in a HS slave */ + if (RecoveryInProgress()) + elog(ERROR, "cannot assign OIDs during recovery"); + LWLockAcquire(OidGenLock, LW_EXCLUSIVE); /* diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 2c32723be62a06f829b11101efed1a9b9eb45a7f..044afd582dd464ffd807663832c729591a4cd287 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.287 2010/02/17 04:19:39 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.288 2010/02/20 21:24:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -402,9 +402,6 @@ AssignTransactionId(TransactionState s) bool isSubXact = (s->parent != NULL); ResourceOwner currentOwner; - if (RecoveryInProgress()) - elog(ERROR, "cannot assign TransactionIds during recovery"); - /* Assert that caller didn't screw up */ Assert(!TransactionIdIsValid(s->transactionId)); Assert(s->state == TRANS_INPROGRESS); diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index e3f528fc4a008cc9ad3d7b1ec27ddfed366576ab..ac68b9639315899f28497eeb77f880654e029a28 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -13,7 +13,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.123 2010/02/14 18:42:13 rhaas Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.124 2010/02/20 21:24:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3017,6 +3017,21 @@ InitTempTableNamespace(void) errmsg("permission denied to create temporary tables in database \"%s\"", get_database_name(MyDatabaseId)))); + /* + * Do not allow a Hot Standby slave session to make temp tables. Aside + * from problems with modifying the system catalogs, there is a naming + * conflict: pg_temp_N belongs to the session with BackendId N on the + * master, not to a slave session with the same BackendId. We should + * not be able to get here anyway due to XactReadOnly checks, but let's + * just make real sure. Note that this also backstops various operations + * that allow XactReadOnly transactions to modify temp tables; they'd need + * RecoveryInProgress checks if not for this. + */ + if (RecoveryInProgress()) + ereport(ERROR, + (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), + errmsg("cannot create temporary tables during recovery"))); + snprintf(namespaceName, sizeof(namespaceName), "pg_temp_%d", MyBackendId); namespaceId = GetSysCacheOid1(NAMESPACENAME, diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 8a31182c99747e16a533537d9fe6a82b57a69213..c7b60de32a9c5ea138aa991c254ac1f25ced22d4 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.153 2010/02/17 16:54:06 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.154 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -534,6 +534,9 @@ pg_notify(PG_FUNCTION_ARGS) else payload = text_to_cstring(PG_GETARG_TEXT_PP(1)); + /* For NOTIFY as a statement, this is checked in ProcessUtility */ + PreventCommandDuringRecovery("NOTIFY"); + Async_Notify(channel, payload); PG_RETURN_VOID(); diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 1ed287306a928514c5791406b5ecc52b80d59438..fbcc4afb968f897b02f585c59a42b929df6661d1 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.324 2010/02/08 04:33:53 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.325 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1023,9 +1023,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString) /* check read-only transaction */ if (XactReadOnly && is_from && !cstate->rel->rd_islocaltemp) - ereport(ERROR, - (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), - errmsg("transaction is read-only"))); + PreventCommandIfReadOnly("COPY FROM"); /* Don't allow COPY w/ OIDs to or from a table without them */ if (cstate->oids && !cstate->rel->rd_rel->relhasoids) diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index f40365ac8fcc1eff6085e34ddaeb539b2d67630d..31096e0beb666214a7567ed739aaa6d27686f5b7 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.27 2010/01/02 16:57:37 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.28 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -55,7 +55,7 @@ LockTableCommand(LockStmt *lockstmt) * This test must match the restrictions defined in LockAcquire() */ if (lockstmt->mode > RowExclusiveLock) - PreventCommandDuringRecovery(); + PreventCommandDuringRecovery("LOCK TABLE"); LockTableRecurse(reloid, relation, lockstmt->mode, lockstmt->nowait, recurse); diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index ffb7fcaba9dc0d5330701f6455c70adc1881d27b..fc2446997a7fcd27dbd2922c82536146f0b025f4 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.167 2010/02/19 06:29:19 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.168 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -459,9 +459,6 @@ nextval_internal(Oid relid) rescnt = 0; bool logit = false; - /* nextval() writes to database and must be prevented during recovery */ - PreventCommandDuringRecovery(); - /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); @@ -472,6 +469,10 @@ nextval_internal(Oid relid) errmsg("permission denied for sequence %s", RelationGetRelationName(seqrel)))); + /* read-only transactions may only modify temp sequences */ + if (!seqrel->rd_islocaltemp) + PreventCommandIfReadOnly("nextval()"); + if (elm->last != elm->cached) /* some numbers were cached */ { Assert(elm->last_valid); @@ -736,9 +737,6 @@ do_setval(Oid relid, int64 next, bool iscalled) Buffer buf; Form_pg_sequence seq; - /* setval() writes to database and must be prevented during recovery */ - PreventCommandDuringRecovery(); - /* open and AccessShareLock sequence */ init_sequence(relid, &elm, &seqrel); @@ -748,6 +746,10 @@ do_setval(Oid relid, int64 next, bool iscalled) errmsg("permission denied for sequence %s", RelationGetRelationName(seqrel)))); + /* read-only transactions may only modify temp sequences */ + if (!seqrel->rd_islocaltemp) + PreventCommandIfReadOnly("setval()"); + /* lock page' buffer and read tuple */ seq = read_info(elm, seqrel, &buf); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index bb7831231271175e7e084aad4072d827465b60da..20d59f9a8c921d316a8875590ca0667b49c44ab2 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.346 2010/02/09 21:43:30 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.347 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -50,6 +50,7 @@ #include "storage/bufmgr.h" #include "storage/lmgr.h" #include "storage/smgr.h" +#include "tcop/utility.h" #include "utils/acl.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -568,6 +569,10 @@ ExecCheckRTEPerms(RangeTblEntry *rte) /* * Check that the query does not imply any writes to non-temp tables. + * + * Note: in a Hot Standby slave this would need to reject writes to temp + * tables as well; but an HS slave can't have created any temp tables + * in the first place, so no need to check that. */ static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt) @@ -577,10 +582,11 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt) /* * CREATE TABLE AS or SELECT INTO? * - * XXX should we allow this if the destination is temp? + * XXX should we allow this if the destination is temp? Considering + * that it would still require catalog changes, probably not. */ if (plannedstmt->intoClause != NULL) - goto fail; + PreventCommandIfReadOnly(CreateCommandTag((Node *) plannedstmt)); /* Fail if write permissions are requested on any non-temp table */ foreach(l, plannedstmt->rtable) @@ -596,15 +602,8 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt) if (isTempNamespace(get_rel_namespace(rte->relid))) continue; - goto fail; + PreventCommandIfReadOnly(CreateCommandTag((Node *) plannedstmt)); } - - return; - -fail: - ereport(ERROR, - (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), - errmsg("transaction is read-only"))); } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 07f4d0c57adb23f3bfc843ef6561ac2dc8b140be..6dc5a51cd2a0beed83bb660ac060a6260dd89eb0 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.333 2010/02/16 22:34:50 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.334 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -151,7 +151,8 @@ check_xact_readonly(Node *parsetree) /* * Note: Commands that need to do more complicated checking are handled * elsewhere, in particular COPY and plannable statements do their own - * checking. + * checking. However they should all call PreventCommandIfReadOnly to + * actually throw the error. */ switch (nodeTag(parsetree)) @@ -217,9 +218,7 @@ check_xact_readonly(Node *parsetree) case T_AlterUserMappingStmt: case T_DropUserMappingStmt: case T_AlterTableSpaceOptionsStmt: - ereport(ERROR, - (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), - errmsg("transaction is read-only"))); + PreventCommandIfReadOnly(CreateCommandTag(parsetree)); break; default: /* do nothing */ @@ -227,6 +226,41 @@ check_xact_readonly(Node *parsetree) } } +/* + * PreventCommandIfReadOnly: throw error if XactReadOnly + * + * This is useful mainly to ensure consistency of the error message wording; + * most callers have checked XactReadOnly for themselves. + */ +void +PreventCommandIfReadOnly(const char *cmdname) +{ + if (XactReadOnly) + ereport(ERROR, + (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), + /* translator: %s is name of a SQL command, eg CREATE */ + errmsg("cannot execute %s in a read-only transaction", + cmdname))); +} + +/* + * PreventCommandDuringRecovery: throw error if RecoveryInProgress + * + * The majority of operations that are unsafe in a Hot Standby slave + * will be rejected by XactReadOnly tests. However there are a few + * commands that are allowed in "read-only" xacts but cannot be allowed + * in Hot Standby mode. Those commands should call this function. + */ +void +PreventCommandDuringRecovery(const char *cmdname) +{ + if (RecoveryInProgress()) + ereport(ERROR, + (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), + /* translator: %s is name of a SQL command, eg CREATE */ + errmsg("cannot execute %s during recovery", + cmdname))); +} /* * CheckRestrictedOperation: throw error for hazardous command if we're @@ -350,7 +384,7 @@ standard_ProcessUtility(Node *parsetree, break; case TRANS_STMT_PREPARE: - PreventCommandDuringRecovery(); + PreventCommandDuringRecovery("PREPARE TRANSACTION"); if (!PrepareTransactionBlock(stmt->gid)) { /* report unsuccessful commit in completionTag */ @@ -360,14 +394,14 @@ standard_ProcessUtility(Node *parsetree, break; case TRANS_STMT_COMMIT_PREPARED: - PreventCommandDuringRecovery(); PreventTransactionChain(isTopLevel, "COMMIT PREPARED"); + PreventCommandDuringRecovery("COMMIT PREPARED"); FinishPreparedTransaction(stmt->gid, true); break; case TRANS_STMT_ROLLBACK_PREPARED: - PreventCommandDuringRecovery(); PreventTransactionChain(isTopLevel, "ROLLBACK PREPARED"); + PreventCommandDuringRecovery("ROLLBACK PREPARED"); FinishPreparedTransaction(stmt->gid, false); break; @@ -744,7 +778,6 @@ standard_ProcessUtility(Node *parsetree, break; case T_GrantStmt: - PreventCommandDuringRecovery(); ExecuteGrantStmt((GrantStmt *) parsetree); break; @@ -927,7 +960,7 @@ standard_ProcessUtility(Node *parsetree, { NotifyStmt *stmt = (NotifyStmt *) parsetree; - PreventCommandDuringRecovery(); + PreventCommandDuringRecovery("NOTIFY"); Async_Notify(stmt->conditionname, stmt->payload); } break; @@ -936,7 +969,7 @@ standard_ProcessUtility(Node *parsetree, { ListenStmt *stmt = (ListenStmt *) parsetree; - PreventCommandDuringRecovery(); + PreventCommandDuringRecovery("LISTEN"); CheckRestrictedOperation("LISTEN"); Async_Listen(stmt->conditionname); } @@ -946,7 +979,7 @@ standard_ProcessUtility(Node *parsetree, { UnlistenStmt *stmt = (UnlistenStmt *) parsetree; - PreventCommandDuringRecovery(); + PreventCommandDuringRecovery("UNLISTEN"); CheckRestrictedOperation("UNLISTEN"); if (stmt->conditionname) Async_Unlisten(stmt->conditionname); @@ -966,12 +999,14 @@ standard_ProcessUtility(Node *parsetree, break; case T_ClusterStmt: - PreventCommandDuringRecovery(); + /* we choose to allow this during "read only" transactions */ + PreventCommandDuringRecovery("CLUSTER"); cluster((ClusterStmt *) parsetree, isTopLevel); break; case T_VacuumStmt: - PreventCommandDuringRecovery(); + /* we choose to allow this during "read only" transactions */ + PreventCommandDuringRecovery("VACUUM"); vacuum((VacuumStmt *) parsetree, InvalidOid, true, NULL, false, isTopLevel); break; @@ -1099,14 +1134,15 @@ standard_ProcessUtility(Node *parsetree, * using various forms of replication. */ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT | - (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE)); + (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE)); break; case T_ReindexStmt: { ReindexStmt *stmt = (ReindexStmt *) parsetree; - PreventCommandDuringRecovery(); + /* we choose to allow this during "read only" transactions */ + PreventCommandDuringRecovery("REINDEX"); switch (stmt->kind) { case OBJECT_INDEX: @@ -2630,12 +2666,3 @@ GetCommandLogLevel(Node *parsetree) return lev; } - -void -PreventCommandDuringRecovery(void) -{ - if (RecoveryInProgress()) - ereport(ERROR, - (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION), - errmsg("cannot be executed during recovery"))); -} diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c index c5bc0e2e481ad2a1d00cfdf15f76e95a2308c265..31c78182017824973ab54d23514338083e9f12c9 100644 --- a/src/backend/utils/adt/txid.c +++ b/src/backend/utils/adt/txid.c @@ -14,7 +14,7 @@ * Author: Jan Wieck, Afilias USA INC. * 64-bit txids: Marko Kreen, Skype Technologies * - * $PostgreSQL: pgsql/src/backend/utils/adt/txid.c,v 1.11 2010/01/07 04:53:34 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/txid.c,v 1.12 2010/02/20 21:24:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -336,7 +336,7 @@ txid_current(PG_FUNCTION_ARGS) * return a valid current xid, so we should not change * this to return NULL or similar invalid xid. */ - PreventCommandDuringRecovery(); + PreventCommandDuringRecovery("txid_current()"); load_xid_epoch(&state); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 2face3a3bdb5fdd7f04b131a4f46b34f80e978f0..d7a80b11d29cf869a51038c8f5c21e1cbf2f732c 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -13,7 +13,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.218 2010/02/07 20:48:13 tgl Exp $ + * $PostgreSQL: pgsql/src/include/miscadmin.h,v 1.219 2010/02/20 21:24:02 tgl Exp $ * * NOTES * some of the information in this file should be moved to other files. @@ -237,7 +237,8 @@ extern bool VacuumCostActive; extern void check_stack_depth(void); /* in tcop/utility.c */ -extern void PreventCommandDuringRecovery(void); +extern void PreventCommandIfReadOnly(const char *cmdname); +extern void PreventCommandDuringRecovery(const char *cmdname); /* in utils/misc/guc.c */ extern int trace_recovery_messages; diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index fc98f0163a0c6a31bac472ca02e97de1da14d689..c4f8965fd1e27b07af41a11371cf1981268374ca 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -45,9 +45,9 @@ CREATE TABLE writetest (a int); CREATE TEMPORARY TABLE temptest (a int); SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY; DROP TABLE writetest; -- fail -ERROR: transaction is read-only +ERROR: cannot execute DROP TABLE in a read-only transaction INSERT INTO writetest VALUES (1); -- fail -ERROR: transaction is read-only +ERROR: cannot execute INSERT in a read-only transaction SELECT * FROM writetest; -- ok a --- @@ -57,14 +57,14 @@ DELETE FROM temptest; -- ok UPDATE temptest SET a = 0 FROM writetest WHERE temptest.a = 1 AND writetest.a = temptest.a; -- ok PREPARE test AS UPDATE writetest SET a = 0; -- ok EXECUTE test; -- fail -ERROR: transaction is read-only +ERROR: cannot execute UPDATE in a read-only transaction SELECT * FROM writetest, temptest; -- ok a | a ---+--- (0 rows) CREATE TABLE test AS SELECT * FROM writetest; -- fail -ERROR: transaction is read-only +ERROR: cannot execute SELECT INTO in a read-only transaction START TRANSACTION READ WRITE; DROP TABLE writetest; -- ok COMMIT;