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).

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 307fb6b6afbb1188b4b9dd09c50b09712fa21871..43aec83ce327c9f0060350d4c0065a16ae32286c 100644 (file)
--- 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 ba2aae576683bdda6fc5a183b6131d2762179c2f..2ff3aae6f8adb49de4aea50aacd169f0da0c6707 100644 (file)
--- 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 38e3211ae72bd91ed46a20f89aa585f29b335a4f..585e55ef6bb54bb78d88778989436347d0593464 100644 (file)
--- 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");
        }
 
        /*