Fix places that were using IsTransactionBlock() as an (inadequate) check

that they'd get to commit immediately on finishing.  There's now a
centralized routine PreventTransactionChain() that implements the
necessary tests.

9 files changed, 107 insertions, 87 deletions

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 046486a6ccc..4687578fedd 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.133 2002/10/21 19:46:45 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.134 2002/10/21 22:06:18 tgl Exp $
  *
  * NOTES
  *		Transaction aborts can now occur two ways:
@@ -1280,9 +1280,12 @@ CommitTransactionCommand(bool forceCommit)
 			 *
 			 * Autocommit mode is forced by either a true forceCommit
 			 * parameter to me, or a true preventChain parameter to the
-			 * preceding StartTransactionCommand call.	This is needed so
-			 * that commands like VACUUM can ensure that the right things
-			 * happen.
+			 * preceding StartTransactionCommand call, or a
+			 * PreventTransactionChain call during the transaction.
+			 * (The parameters could be omitted, but it turns out most
+			 * callers of StartTransactionCommand/CommitTransactionCommand
+			 * want to force autocommit, so making them all call
+			 * PreventTransactionChain would just be extra notation.)
 			 */
 		case TBLOCK_DEFAULT:
 			if (autocommit || forceCommit || suppressChain)
@@ -1429,6 +1432,60 @@ AbortCurrentTransaction(void)
 	}
 }
 
+/* --------------------------------
+ *		PreventTransactionChain
+ *
+ * This routine is to be called by statements that must not run inside
+ * a transaction block, typically because they have non-rollback-able
+ * side effects or do internal commits.
+ *
+ * If we have already started a transaction block, issue an error; also issue
+ * an error if we appear to be running inside a user-defined function (which
+ * could issue more commands and possibly cause a failure after the statement
+ * completes).  In autocommit-off mode, we allow the statement if a block is
+ * not already started, and force the statement to be autocommitted despite
+ * the mode.
+ *
+ * stmtNode: pointer to parameter block for statement; this is used in
+ * a very klugy way to determine whether we are inside a function.
+ * stmtType: statement type name for error messages.
+ * --------------------------------
+ */
+void
+PreventTransactionChain(void *stmtNode, const char *stmtType)
+{
+	/*
+	 * xact block already started?
+	 */
+	if (IsTransactionBlock())
+	{
+		/* translator: %s represents an SQL statement name */
+		elog(ERROR, "%s cannot run inside a transaction block", stmtType);
+	}
+	/*
+	 * Are we inside a function call?  If the statement's parameter block
+	 * was allocated in QueryContext, assume it is an interactive command.
+	 * Otherwise assume it is coming from a function.
+	 */
+	if (!MemoryContextContains(QueryContext, stmtNode))
+	{
+		/* translator: %s represents an SQL statement name */
+		elog(ERROR, "%s cannot be executed from a function", stmtType);
+	}
+	/* If we got past IsTransactionBlock test, should be in default state */
+	if (CurrentTransactionState->blockState != TBLOCK_DEFAULT)
+		elog(ERROR, "PreventTransactionChain: can't prevent chain");
+	/* okay to set the flag */
+	suppressChain = true;
+	/* If we're in autocommit-off node, generate a notice */
+	if (!autocommit)
+	{
+		/* translator: %s represents an SQL statement name */
+		elog(NOTICE, "%s will be committed automatically", stmtType);
+	}
+}
+
+
 /* ----------------------------------------------------------------
  *					   transaction block support
  * ----------------------------------------------------------------
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index dea79dc4d6e..b476a1181ea 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.231 2002/09/22 19:42:50 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/heap.c,v 1.232 2002/10/21 22:06:18 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1917,20 +1917,9 @@ heap_truncate(Oid rid)
 	Relation	rel;
 
 	/* Open relation for processing, and grab exclusive access on it. */
-
 	rel = heap_open(rid, AccessExclusiveLock);
 
 	/*
-	 * TRUNCATE TABLE within a transaction block is dangerous, because if
-	 * the transaction is later rolled back we have no way to undo
-	 * truncation of the relation's physical file.  Disallow it except for
-	 * a rel created in the current xact (which would be deleted on abort,
-	 * anyway).
-	 */
-	if (IsTransactionBlock() && !rel->rd_isnew)
-		elog(ERROR, "TRUNCATE TABLE cannot run inside a transaction block");
-
-	/*
 	 * Release any buffers associated with this relation.  If they're
 	 * dirty, they're just dropped without bothering to flush to disk.
 	 */
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 696f33bbfe5..0c8f5327c96 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.201 2002/09/27 15:05:23 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.202 2002/10/21 22:06:19 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1741,16 +1741,6 @@ reindex_index(Oid indexId, bool force, bool inplace)
 	bool		old;
 
 	/*
-	 * REINDEX within a transaction block is dangerous, because if the
-	 * transaction is later rolled back we have no way to undo truncation
-	 * of the index's physical file.  Disallow it.
-	 *
-	 * XXX if we're not doing an inplace rebuild, wouldn't this be okay?
-	 */
-	if (IsTransactionBlock())
-		elog(ERROR, "REINDEX cannot run inside a transaction block");
-
-	/*
 	 * Open our index relation and get an exclusive lock on it.
 	 *
 	 * Note: doing this before opening the parent heap relation means there's
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index addb6dcae4e..327aac29593 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/dbcommands.c,v 1.105 2002/09/04 20:31:15 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/dbcommands.c,v 1.106 2002/10/21 22:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -152,8 +152,7 @@ createdb(const CreatedbStmt *stmt)
 	}
 
 	/* don't call this in a transaction block */
-	if (IsTransactionBlock())
-		elog(ERROR, "CREATE DATABASE: may not be called in a transaction block");
+	PreventTransactionChain((void *) stmt, "CREATE DATABASE");
 
 	/*
 	 * Check for db name conflict.	There is a race condition here, since
@@ -382,8 +381,7 @@ dropdb(const char *dbname)
 	if (strcmp(dbname, DatabaseName) == 0)
 		elog(ERROR, "DROP DATABASE: cannot be executed on the currently open database");
 
-	if (IsTransactionBlock())
-		elog(ERROR, "DROP DATABASE: may not be called in a transaction block");
+	PreventTransactionChain((void *) dbname, "DROP DATABASE");
 
 	/*
 	 * Obtain exclusive lock on pg_database.  We need this to ensure that
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 43aec83ce32..5447780b69e 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.91 2002/10/19 20:15:08 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.92 2002/10/21 22:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -560,15 +560,10 @@ ReindexIndex(RangeVar *indexRelation, bool force /* currently unused */ )
 {
 	Oid			indOid;
 	HeapTuple	tuple;
-	bool		overwrite = false;
+	bool		overwrite;
 
-	/*
-	 * REINDEX within a transaction block is dangerous, because if the
-	 * transaction is later rolled back we have no way to undo truncation
-	 * of the index's physical file.  Disallow it.
-	 */
-	if (IsTransactionBlock())
-		elog(ERROR, "REINDEX cannot run inside a BEGIN/END block");
+	/* Choose in-place-or-not mode */
+	overwrite = IsIgnoringSystemIndexes();
 
 	indOid = RangeVarGetRelid(indexRelation, false);
 	tuple = SearchSysCache(RELOID,
@@ -595,8 +590,14 @@ ReindexIndex(RangeVar *indexRelation, bool force /* currently unused */ )
 
 	ReleaseSysCache(tuple);
 
-	if (IsIgnoringSystemIndexes())
-		overwrite = true;
+	/*
+	 * In-place REINDEX within a transaction block is dangerous, because
+	 * if the transaction is later rolled back we have no way to undo
+	 * truncation of the index's physical file.  Disallow it.
+	 */
+	if (overwrite)
+		PreventTransactionChain((void *) indexRelation, "REINDEX");
+
 	if (!reindex_index(indOid, force, overwrite))
 		elog(WARNING, "index \"%s\" wasn't reindexed", indexRelation->relname);
 }
@@ -611,14 +612,6 @@ ReindexTable(RangeVar *relation, bool force)
 	Oid			heapOid;
 	char		relkind;
 
-	/*
-	 * REINDEX within a transaction block is dangerous, because if the
-	 * transaction is later rolled back we have no way to undo truncation
-	 * of the index's physical file.  Disallow it.
-	 */
-	if (IsTransactionBlock())
-		elog(ERROR, "REINDEX cannot run inside a BEGIN/END block");
-
 	heapOid = RangeVarGetRelid(relation, false);
 	relkind = get_rel_relkind(heapOid);
 
@@ -626,6 +619,17 @@ ReindexTable(RangeVar *relation, bool force)
 		elog(ERROR, "relation \"%s\" is of type \"%c\"",
 			 relation->relname, relkind);
 
+	/*
+	 * In-place REINDEX within a transaction block is dangerous, because
+	 * if the transaction is later rolled back we have no way to undo
+	 * truncation of the index's physical file.  Disallow it.
+	 *
+	 * XXX we assume that in-place reindex will only be done if
+	 * IsIgnoringSystemIndexes() is true.
+	 */
+	if (IsIgnoringSystemIndexes())
+		PreventTransactionChain((void *) relation, "REINDEX");
+
 	if (!reindex_relation(heapOid, force))
 		elog(WARNING, "table \"%s\" wasn't reindexed", relation->relname);
 }
@@ -666,12 +670,7 @@ ReindexDatabase(const char *dbname, bool force, bool all)
 	 * transaction, then our commit- and start-transaction-command calls
 	 * would not have the intended effect!
 	 */
-	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");
+	PreventTransactionChain((void *) dbname, "REINDEX");
 
 	/*
 	 * Create a memory context that will survive forced transaction
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ae15b815332..f211448f413 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.49 2002/10/21 20:31:51 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.50 2002/10/21 22:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -376,6 +376,16 @@ TruncateRelation(const RangeVar *relation)
 		aclcheck_error(ACLCHECK_NOT_OWNER, RelationGetRelationName(rel));
 
 	/*
+	 * Truncate within a transaction block is dangerous, because if
+	 * the transaction is later rolled back we have no way to undo
+	 * truncation of the relation's physical file.  Disallow it except for
+	 * a rel created in the current xact (which would be deleted on abort,
+	 * anyway).
+	 */
+	if (!rel->rd_isnew)
+		PreventTransactionChain((void *) relation, "TRUNCATE TABLE");
+
+	/*
 	 * Don't allow truncate on temp tables of other backends ... their
 	 * local buffer manager is not going to cope.
 	 */
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2ff3aae6f8a..160289c91e0 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.242 2002/10/19 20:15:09 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.243 2002/10/21 22:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -177,12 +177,8 @@ vacuum(VacuumStmt *vacstmt)
 	 * user's transaction too, which would certainly not be the desired
 	 * behavior.
 	 */
-	if (vacstmt->vacuum && IsTransactionBlock())
-		elog(ERROR, "%s cannot run inside a BEGIN/END block", stmttype);
-
-	/* Running VACUUM from a function would free the function context */
-	if (vacstmt->vacuum && !MemoryContextContains(QueryContext, vacstmt))
-		elog(ERROR, "%s cannot be executed from a function", stmttype);
+	if (vacstmt->vacuum)
+		PreventTransactionChain((void *) vacstmt, stmttype);
 
 	/*
 	 * Send info about dead objects to the statistics collector
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index c8ed3ed8cb2..30c8e8f15b9 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.252 2002/10/20 00:31:53 tgl Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.253 2002/10/21 22:06:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1631,16 +1631,6 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 		if (stmt->forUpdate)
 			elog(ERROR, "DECLARE/UPDATE is not supported"
 				 "\n\tCursors must be READ ONLY");
-
-		/*
-		 * 15 august 1991 -- since 3.0 postgres does locking right, we
-		 * discovered that portals were violating locking protocol. portal
-		 * locks cannot span xacts. as a short-term fix, we installed the
-		 * check here. -- mao
-		 */
-		if (!IsTransactionBlock())
-			elog(ERROR, "DECLARE CURSOR may only be used in begin/end transaction blocks");
-
 		qry->into = makeNode(RangeVar);
 		qry->into->relname = stmt->portalname;
 		qry->isPortal = TRUE;
@@ -1849,16 +1839,6 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 		if (forUpdate)
 			elog(ERROR, "DECLARE/UPDATE is not supported"
 				 "\n\tCursors must be READ ONLY");
-
-		/*
-		 * 15 august 1991 -- since 3.0 postgres does locking right, we
-		 * discovered that portals were violating locking protocol. portal
-		 * locks cannot span xacts. as a short-term fix, we installed the
-		 * check here. -- mao
-		 */
-		if (!IsTransactionBlock())
-			elog(ERROR, "DECLARE CURSOR may only be used in begin/end transaction blocks");
-
 		qry->into = makeNode(RangeVar);
 		qry->into->relname = portalname;
 		qry->isPortal = TRUE;
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index b74b9d4cdca..e16771231d3 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: xact.h,v 1.45 2002/08/30 22:18:07 tgl Exp $
+ * $Id: xact.h,v 1.46 2002/10/21 22:06:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -113,6 +113,7 @@ extern void EndTransactionBlock(void);
 extern bool IsTransactionBlock(void);
 extern void UserAbortTransactionBlock(void);
 extern void AbortOutOfAnyTransaction(void);
+extern void PreventTransactionChain(void *stmtNode, const char *stmtType);
 
 extern void RecordTransactionCommit(void);