Fix failure to ensure that a snapshot is available to datatype input functions

when they are invoked by the parser.  We had been setting up a snapshot at
plan time but really it needs to be done earlier, before parse analysis.
Per report from Dmitry Koterov.

Also fix two related problems discovered while poking at this one:
exec_bind_message called datatype input functions without establishing a
snapshot, and SET CONSTRAINTS IMMEDIATE could call trigger functions without
establishing a snapshot.

Backpatch to 8.2.  The underlying problem goes much further back, but it is
masked in 8.1 and before because we didn't attempt to invoke domain check
constraints within datatype input.  It would only be exposed if a C-language
datatype input function used the snapshot; which evidently none do, or we'd
have heard complaints sooner.  Since this code has changed a lot over time,
a back-patch is hardly risk-free, and so I'm disinclined to patch further
than absolutely necessary.

1 files changed, 76 insertions, 84 deletions

diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index bbfdd888c1c..5019df6f86a 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -33,7 +33,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.15.2.1 2008/09/15 23:37:49 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.15.2.2 2008/12/13 02:00:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -70,7 +70,6 @@ static List *cached_plans_list = NIL;
 
 static void StoreCachedPlan(CachedPlanSource *plansource, List *stmt_list,
 				MemoryContext plan_context);
-static List *do_planning(List *querytrees, int cursorOptions);
 static void AcquireExecutorLocks(List *stmt_list, bool acquire);
 static void AcquirePlannerLocks(List *stmt_list, bool acquire);
 static void LockRelid(Oid relid, LOCKMODE lockmode, void *arg);
@@ -457,8 +456,8 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
 	 */
 	if (!plan)
 	{
+		Snapshot	saveActiveSnapshot = ActiveSnapshot;
 		List	   *slist;
-		TupleDesc	resultDesc;
 
 		/*
 		 * Restore the search_path that was in use when the plan was made.
@@ -467,50 +466,86 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
 		PushOverrideSearchPath(plansource->search_path);
 
 		/*
-		 * Run parse analysis and rule rewriting.  The parser tends to
-		 * scribble on its input, so we must copy the raw parse tree to
-		 * prevent corruption of the cache.  Note that we do not use
-		 * parse_analyze_varparams(), assuming that the caller never wants the
-		 * parameter types to change from the original values.
+		 * If a snapshot is already set (the normal case), we can just use
+		 * that for parsing/planning.  But if it isn't, install one.  We must
+		 * arrange to restore ActiveSnapshot afterward, to ensure that
+		 * RevalidateCachedPlan has no caller-visible effects on the
+		 * snapshot.  Having to replan is an unusual case, and it seems a
+		 * really bad idea for RevalidateCachedPlan to affect the snapshot
+		 * only in unusual cases.  (Besides, the snap might have been created
+		 * in a short-lived context.)
 		 */
-		slist = pg_analyze_and_rewrite(copyObject(plansource->raw_parse_tree),
-									   plansource->query_string,
-									   plansource->param_types,
-									   plansource->num_params);
-
-		if (plansource->fully_planned)
+		PG_TRY();
 		{
-			/* Generate plans for queries */
-			slist = do_planning(slist, plansource->cursor_options);
-		}
+			Snapshot	mySnapshot = NULL;
+			TupleDesc	resultDesc;
 
-		/*
-		 * Check or update the result tupdesc.	XXX should we use a weaker
-		 * condition than equalTupleDescs() here?
-		 */
-		resultDesc = PlanCacheComputeResultDesc(slist);
-		if (resultDesc == NULL && plansource->resultDesc == NULL)
-		{
-			/* OK, doesn't return tuples */
+			if (ActiveSnapshot == NULL)
+			{
+				mySnapshot = CopySnapshot(GetTransactionSnapshot());
+				ActiveSnapshot = mySnapshot;
+			}
+
+			/*
+			 * Run parse analysis and rule rewriting.  The parser tends to
+			 * scribble on its input, so we must copy the raw parse tree to
+			 * prevent corruption of the cache.  Note that we do not use
+			 * parse_analyze_varparams(), assuming that the caller never wants
+			 * the parameter types to change from the original values.
+			 */
+			slist = pg_analyze_and_rewrite(copyObject(plansource->raw_parse_tree),
+										   plansource->query_string,
+										   plansource->param_types,
+										   plansource->num_params);
+
+			if (plansource->fully_planned)
+			{
+				/* Generate plans for queries */
+				slist = pg_plan_queries(slist, plansource->cursor_options,
+										NULL, false);
+			}
+
+			/*
+			 * Check or update the result tupdesc.	XXX should we use a weaker
+			 * condition than equalTupleDescs() here?
+			 */
+			resultDesc = PlanCacheComputeResultDesc(slist);
+			if (resultDesc == NULL && plansource->resultDesc == NULL)
+			{
+				/* OK, doesn't return tuples */
+			}
+			else if (resultDesc == NULL || plansource->resultDesc == NULL ||
+					 !equalTupleDescs(resultDesc, plansource->resultDesc))
+			{
+				MemoryContext oldcxt;
+
+				/* can we give a better error message? */
+				if (plansource->fixed_result)
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("cached plan must not change result type")));
+				oldcxt = MemoryContextSwitchTo(plansource->context);
+				if (resultDesc)
+					resultDesc = CreateTupleDescCopy(resultDesc);
+				if (plansource->resultDesc)
+					FreeTupleDesc(plansource->resultDesc);
+				plansource->resultDesc = resultDesc;
+				MemoryContextSwitchTo(oldcxt);
+			}
+
+			/* Done with snapshot */
+			if (mySnapshot)
+				FreeSnapshot(mySnapshot);
 		}
-		else if (resultDesc == NULL || plansource->resultDesc == NULL ||
-				 !equalTupleDescs(resultDesc, plansource->resultDesc))
+		PG_CATCH();
 		{
-			MemoryContext oldcxt;
-
-			/* can we give a better error message? */
-			if (plansource->fixed_result)
-				ereport(ERROR,
-						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("cached plan must not change result type")));
-			oldcxt = MemoryContextSwitchTo(plansource->context);
-			if (resultDesc)
-				resultDesc = CreateTupleDescCopy(resultDesc);
-			if (plansource->resultDesc)
-				FreeTupleDesc(plansource->resultDesc);
-			plansource->resultDesc = resultDesc;
-			MemoryContextSwitchTo(oldcxt);
+			/* Restore global vars and propagate error */
+			ActiveSnapshot = saveActiveSnapshot;
+			PG_RE_THROW();
 		}
+		PG_END_TRY();
+
+		ActiveSnapshot = saveActiveSnapshot;
 
 		/* Now we can restore current search path */
 		PopOverrideSearchPath();
@@ -537,49 +572,6 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner)
 }
 
 /*
- * Invoke the planner on some rewritten queries.  This is broken out of
- * RevalidateCachedPlan just to avoid plastering "volatile" all over that
- * function's variables.
- */
-static List *
-do_planning(List *querytrees, int cursorOptions)
-{
-	List	   *stmt_list;
-
-	/*
-	 * If a snapshot is already set (the normal case), we can just use that
-	 * for planning.  But if it isn't, we have to tell pg_plan_queries to make
-	 * a snap if it needs one.	In that case we should arrange to reset
-	 * ActiveSnapshot afterward, to ensure that RevalidateCachedPlan has no
-	 * caller-visible effects on the snapshot.	Having to replan is an unusual
-	 * case, and it seems a really bad idea for RevalidateCachedPlan to affect
-	 * the snapshot only in unusual cases.	(Besides, the snap might have been
-	 * created in a short-lived context.)
-	 */
-	if (ActiveSnapshot != NULL)
-		stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, false);
-	else
-	{
-		PG_TRY();
-		{
-			stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, true);
-		}
-		PG_CATCH();
-		{
-			/* Restore global vars and propagate error */
-			ActiveSnapshot = NULL;
-			PG_RE_THROW();
-		}
-		PG_END_TRY();
-
-		ActiveSnapshot = NULL;
-	}
-
-	return stmt_list;
-}
-
-
-/*
  * ReleaseCachedPlan: release active use of a cached plan.
  *
  * This decrements the reference count, and frees the plan if the count