From 60992456ed00b52f90e47a736ace12668b881225 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 19 Oct 2002 20:15:09 +0000 Subject: [PATCH] Invert logic in pg_exec_query_string() so that we set a snapshot for all utility statement types *except* a short list, per discussion a few days ago. Add missing SetQuerySnapshot calls in VACUUM and REINDEX, and guard against calling REINDEX DATABASE from a function (has same problem as VACUUM). --- src/backend/commands/indexcmds.c | 7 +++++- src/backend/commands/vacuum.c | 6 ++++- src/backend/tcop/postgres.c | 38 ++++++++++++++++++++++---------- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 307fb6b6af..43aec83ce3 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.90 2002/09/23 00:42:48 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.91 2002/10/19 20:15:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -669,6 +669,10 @@ ReindexDatabase(const char *dbname, bool force, bool all) if (IsTransactionBlock()) elog(ERROR, "REINDEX DATABASE cannot run inside a BEGIN/END block"); + /* Running this from a function would free the function context */ + if (!MemoryContextContains(QueryContext, (void *) dbname)) + elog(ERROR, "REINDEX DATABASE cannot be executed from a function"); + /* * Create a memory context that will survive forced transaction * commits we do below. Since it is a child of QueryContext, it will @@ -724,6 +728,7 @@ ReindexDatabase(const char *dbname, bool force, bool all) for (i = 0; i < relcnt; i++) { StartTransactionCommand(true); + SetQuerySnapshot(); /* might be needed for functional index */ if (reindex_relation(relids[i], force)) elog(NOTICE, "relation %u was reindexed", relids[i]); CommitTransactionCommand(true); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index ba2aae5766..2ff3aae6f8 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.241 2002/09/27 20:57:08 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.242 2002/10/19 20:15:09 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -306,7 +306,10 @@ vacuum(VacuumStmt *vacstmt) * multiple tables). */ if (vacstmt->vacuum) + { StartTransactionCommand(true); + SetQuerySnapshot(); /* might be needed for functional index */ + } else old_context = MemoryContextSwitchTo(anl_context); @@ -724,6 +727,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind) /* Begin a transaction for vacuuming this relation */ StartTransactionCommand(true); + SetQuerySnapshot(); /* might be needed for functional index */ /* * Check for user-requested abort. Note we want this to be inside a diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 38e3211ae7..585e55ef6b 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.304 2002/10/18 22:05:35 petere Exp $ + * $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.305 2002/10/19 20:15:09 tgl Exp $ * * NOTES * this is the "main" module of the postgres backend and @@ -756,16 +756,30 @@ pg_exec_query_string(StringInfo query_string, /* string to execute */ elog(DEBUG2, "ProcessUtility"); - /* set snapshot if utility stmt needs one */ - /* XXX maybe cleaner to list those that shouldn't set one? */ - if (IsA(utilityStmt, AlterTableStmt) || - IsA(utilityStmt, ClusterStmt) || - IsA(utilityStmt, CopyStmt) || - IsA(utilityStmt, ExecuteStmt) || - IsA(utilityStmt, ExplainStmt) || - IsA(utilityStmt, IndexStmt) || - IsA(utilityStmt, PrepareStmt) || - IsA(utilityStmt, ReindexStmt)) + /* + * Set snapshot if utility stmt needs one. Most reliable + * way to do this seems to be to enumerate those that do not + * need one; this is a short list. Transaction control, + * LOCK, and SET must *not* set a snapshot since they need + * to be executable at the start of a serializable transaction + * without freezing a snapshot. By extension we allow SHOW + * not to set a snapshot. The other stmts listed are just + * efficiency hacks. Beware of listing anything that can + * modify the database --- if, say, it has to update a + * functional index, then it had better have a snapshot. + */ + if (! (IsA(utilityStmt, TransactionStmt) || + IsA(utilityStmt, LockStmt) || + IsA(utilityStmt, VariableSetStmt) || + IsA(utilityStmt, VariableShowStmt) || + IsA(utilityStmt, VariableResetStmt) || + IsA(utilityStmt, ConstraintsSetStmt) || + /* efficiency hacks from here down */ + IsA(utilityStmt, FetchStmt) || + IsA(utilityStmt, ListenStmt) || + IsA(utilityStmt, NotifyStmt) || + IsA(utilityStmt, UnlistenStmt) || + IsA(utilityStmt, CheckPointStmt))) SetQuerySnapshot(); /* end transaction block if transaction or variable stmt */ @@ -1769,7 +1783,7 @@ PostgresMain(int argc, char *argv[], const char *username) if (!IsUnderPostmaster) { puts("\nPOSTGRES backend interactive interface "); - puts("$Revision: 1.304 $ $Date: 2002/10/18 22:05:35 $\n"); + puts("$Revision: 1.305 $ $Date: 2002/10/19 20:15:09 $\n"); } /*