Avoid crash in estimate_array_length with null root pointer.

tglsfdc · tglsfdc · commit a3c4a91f1e28 · 2024-10-09T17:07:53.000-04:00
Commit 9391f71 added a "PlannerInfo *root" parameter to estimate_array_length, but failed to consider the possibility that NULL would be passed for that, leading to a null pointer dereference. We could rectify the particular case shown in the bug report by fixing simplify_function/inline_function to pass through the root pointer. However, as long as eval_const_expressions is documented to accept NULL for root, similar hazards would remain. For now, let's just do the narrow fix of hardening estimate_array_length to not crash. Its behavior with NULL root will be the same as it was before 9391f71, so this is not too awful. Per report from Fredrik Widlert (via Paul Ramsey). Back-patch to v17 where 9391f71 came in. Discussion: https://postgr.es/m/518339E7-173E-45EC-A0FF-9A4A62AA4F40@cleverelephant.ca
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
@@ -4635,6 +4635,9 @@ cost_rescan(PlannerInfo *root, Path *path,
  *		preferred since it allows caching of the results.)
  *		The result includes both a one-time (startup) component,
  *		and a per-evaluation component.
+ *
+ * Note: in some code paths root can be passed as NULL, resulting in
+ * slightly worse estimates.
  */
 void
 cost_qual_eval(QualCost *cost, List *quals, PlannerInfo *root)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
@@ -2131,6 +2131,9 @@ scalararraysel(PlannerInfo *root,
  *
  * Note: the result is integral, but we use "double" to avoid overflow
  * concerns.  Most callers will use it in double-type expressions anyway.
+ *
+ * Note: in some code paths root can be passed as NULL, resulting in
+ * slightly worse estimates.
  */
 double
 estimate_array_length(PlannerInfo *root, Node *arrayexpr)
@@ -2154,7 +2157,7 @@ estimate_array_length(PlannerInfo *root, Node *arrayexpr)
 	{
 		return list_length(((ArrayExpr *) arrayexpr)->elements);
 	}
-	else if (arrayexpr)
+	else if (arrayexpr && root)
 	{
 		/* See if we can find any statistics about it */
 		VariableStatData vardata;

Original file line number	Diff line number	Diff line change
`@@ -2131,6 +2131,9 @@ scalararraysel(PlannerInfo *root,`
`2131`	`2131`	`*`
`2132`	`2132`	`* Note: the result is integral, but we use "double" to avoid overflow`
`2133`	`2133`	`* concerns. Most callers will use it in double-type expressions anyway.`
	`2134`	`+ *`
	`2135`	`+ * Note: in some code paths root can be passed as NULL, resulting in`
	`2136`	`+ * slightly worse estimates.`
`2134`	`2137`	`*/`
`2135`	`2138`	`double`
`2136`	`2139`	`estimate_array_length(PlannerInfo root, Node arrayexpr)`
`@@ -2154,7 +2157,7 @@ estimate_array_length(PlannerInfo root, Node arrayexpr)`
`2154`	`2157`	`{`
`2155`	`2158`	`return list_length(((ArrayExpr *) arrayexpr)->elements);`
`2156`	`2159`	`}`
`2157`		`- else if (arrayexpr)`
	`2160`	`+ else if (arrayexpr && root)`
`2158`	`2161`	`{`
`2159`	`2162`	`/* See if we can find any statistics about it */`
`2160`	`2163`	`VariableStatData vardata;`