Use a fresh copy of query_list when making a second plan in GetCachedPlan.

tglsfdc · tglsfdc · commit 21fb95da46bc · 2011-09-26T12:44:17.000-04:00
The code path that tried a generic plan, didn't like it, and then made a
custom plan was mistakenly passing the same copy of the query_list to the
planner both times.  This doesn't work too well for nontrivial queries,
since the planner tends to scribble on its input.  Diagnosis and fix by
Yamamoto Takashi.
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
@@ -697,7 +697,8 @@ CheckCachedPlan(CachedPlanSource *plansource)
 /*
  * BuildCachedPlan: construct a new CachedPlan from a CachedPlanSource.
  *
- * qlist should be the result value from a previous RevalidateCachedQuery.
+ * qlist should be the result value from a previous RevalidateCachedQuery,
+ * or it can be set to NIL if we need to re-copy the plansource's query_list.
  *
  * To build a generic, parameter-value-independent plan, pass NULL for
  * boundParams.  To build a custom plan, pass the actual parameter values via
@@ -980,6 +981,13 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
 			 * plan.
 			 */
 			customplan = choose_custom_plan(plansource, boundParams);
+
+			/*
+			 * If we choose to plan again, we need to re-copy the query_list,
+			 * since the planner probably scribbled on it.  We can force
+			 * BuildCachedPlan to do that by passing NIL.
+			 */
+			qlist = NIL;
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -697,7 +697,8 @@ CheckCachedPlan(CachedPlanSource *plansource)`
`697`	`697`	`/*`
`698`	`698`	`* BuildCachedPlan: construct a new CachedPlan from a CachedPlanSource.`
`699`	`699`	`*`
`700`		`- * qlist should be the result value from a previous RevalidateCachedQuery.`
	`700`	`+ * qlist should be the result value from a previous RevalidateCachedQuery,`
	`701`	`+ * or it can be set to NIL if we need to re-copy the plansource's query_list.`
`701`	`702`	`*`
`702`	`703`	`* To build a generic, parameter-value-independent plan, pass NULL for`
`703`	`704`	`* boundParams. To build a custom plan, pass the actual parameter values via`
`@@ -980,6 +981,13 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,`
`980`	`981`	`* plan.`
`981`	`982`	`*/`
`982`	`983`	`customplan = choose_custom_plan(plansource, boundParams);`
	`984`	`+`
	`985`	`+ /*`
	`986`	`+ * If we choose to plan again, we need to re-copy the query_list,`
	`987`	`+ * since the planner probably scribbled on it. We can force`
	`988`	`+ * BuildCachedPlan to do that by passing NIL.`
	`989`	`+ */`
	`990`	`+ qlist = NIL;`
`983`	`991`	`}`
`984`	`992`	`}`
`985`	`993`