Avoid unnecessary snapshot-acquisitions in BuildCachedPlan.

tglsfdc · tglsfdc · commit d5aa7a9fe68b · 2011-09-25T17:34:20.000-04:00
I had copied-and-pasted a claim that we couldn't reach this point when
dealing with utility statements, but that was a leftover from when the
caller was required to supply a plan to start with.  We now will go
through here at least once when handling a utility statement, so it
seems worth a check to see whether a snapshot is actually needed.
(Note that analyze_requires_snapshot is quite a cheap test.)

Per suggestion from Yamamoto Takashi.  I don't think I believe that this
resolves his reported assertion failure; but it's worth changing anyway,
just to save a cycle or two.
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
@@ -54,6 +54,7 @@
 #include "nodes/nodeFuncs.h"
 #include "optimizer/planmain.h"
 #include "optimizer/prep.h"
+#include "parser/analyze.h"
 #include "parser/parsetree.h"
 #include "storage/lmgr.h"
 #include "tcop/pquery.h"
@@ -756,14 +757,12 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
 	PushOverrideSearchPath(plansource->search_path);
 
 	/*
-	 * If a snapshot is already set (the normal case), we can just use
-	 * that for parsing/planning.  But if it isn't, install one.  Note: no
-	 * point in checking whether parse analysis requires a snapshot;
-	 * utility commands don't have invalidatable plans, so we'd not get
-	 * here for such a command.
+	 * If a snapshot is already set (the normal case), we can just use that
+	 * for planning.  But if it isn't, and we need one, install one.
 	 */
 	snapshot_set = false;
-	if (!ActiveSnapshotSet())
+	if (!ActiveSnapshotSet() &&
+		analyze_requires_snapshot(plansource->raw_parse_tree))
 	{
 		PushActiveSnapshot(GetTransactionSnapshot());
 		snapshot_set = true;
