v3-0001-Move-some-things-outside-of-inline_set_returning_.patch

From c74e3fea09f44df39c62299777527e28616e0345 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Tue, 24 Jun 2025 19:10:15 -0700
Subject: [PATCH v3 1/2] Move some things outside of
 inline_set_returning_function.

Added a new inline_sql_set_returning_function in preparation for
inline_set_returning_function_with_support. Then
inline_set_returning_function can call both, handling their shared needs
itself.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
---
 src/backend/optimizer/util/clauses.c | 273 +++++++++++++++------------
 1 file changed, 157 insertions(+), 116 deletions(-)

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 6f0b338d2cd..77f48ff4069 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5146,36 +5146,21 @@ evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod,
 
 
 /*
- * inline_set_returning_function
- *		Attempt to "inline" a set-returning function in the FROM clause.
- *
- * "rte" is an RTE_FUNCTION rangetable entry.  If it represents a call of a
- * set-returning SQL function that can safely be inlined, expand the function
- * and return the substitute Query structure.  Otherwise, return NULL.
+ * inline_sql_set_returning_function
  *
- * We assume that the RTE's expression has already been put through
- * eval_const_expressions(), which among other things will take care of
- * default arguments and named-argument notation.
+ * This implements inline_set_returning_function for sql-language functions.
+ * It parses the body (or uses the pre-parsed body if available).
  *
- * This has a good deal of similarity to inline_function(), but that's
- * for the non-set-returning case, and there are enough differences to
- * justify separate functions.
+ * Returns NULL if the function couldn't be inlined.
  */
-Query *
-inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
+static Query *
+inline_sql_set_returning_function(PlannerInfo *root, RangeTblEntry *rte,
+								  RangeTblFunction *rtfunc,
+								  FuncExpr *fexpr, Oid func_oid, HeapTuple func_tuple,
+								  Form_pg_proc funcform, char *src)
 {
-	RangeTblFunction *rtfunc;
-	FuncExpr   *fexpr;
-	Oid			func_oid;
-	HeapTuple	func_tuple;
-	Form_pg_proc funcform;
-	char	   *src;
-	Datum		tmp;
+	Datum		sqlbody;
 	bool		isNull;
-	MemoryContext oldcxt;
-	MemoryContext mycxt;
-	inline_error_callback_arg callback_arg;
-	ErrorContextCallback sqlerrcontext;
 	SQLFunctionParseInfoPtr pinfo;
 	TypeFuncClass functypclass;
 	TupleDesc	rettupdesc;
@@ -5185,29 +5170,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 
 	Assert(rte->rtekind == RTE_FUNCTION);
 
-	/*
-	 * It doesn't make a lot of sense for a SQL SRF to refer to itself in its
-	 * own FROM clause, since that must cause infinite recursion at runtime.
-	 * It will cause this code to recurse too, so check for stack overflow.
-	 * (There's no need to do more.)
-	 */
-	check_stack_depth();
-
-	/* Fail if the RTE has ORDINALITY - we don't implement that here. */
-	if (rte->funcordinality)
-		return NULL;
-
-	/* Fail if RTE isn't a single, simple FuncExpr */
-	if (list_length(rte->functions) != 1)
-		return NULL;
-	rtfunc = (RangeTblFunction *) linitial(rte->functions);
-
-	if (!IsA(rtfunc->funcexpr, FuncExpr))
-		return NULL;
-	fexpr = (FuncExpr *) rtfunc->funcexpr;
-
-	func_oid = fexpr->funcid;
-
 	/*
 	 * The function must be declared to return a set, else inlining would
 	 * change the results if the contained SELECT didn't return exactly one
@@ -5216,35 +5178,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	if (!fexpr->funcretset)
 		return NULL;
 
-	/*
-	 * Refuse to inline if the arguments contain any volatile functions or
-	 * sub-selects.  Volatile functions are rejected because inlining may
-	 * result in the arguments being evaluated multiple times, risking a
-	 * change in behavior.  Sub-selects are rejected partly for implementation
-	 * reasons (pushing them down another level might change their behavior)
-	 * and partly because they're likely to be expensive and so multiple
-	 * evaluation would be bad.
-	 */
-	if (contain_volatile_functions((Node *) fexpr->args) ||
-		contain_subplans((Node *) fexpr->args))
-		return NULL;
-
-	/* Check permission to call function (fail later, if not) */
-	if (object_aclcheck(ProcedureRelationId, func_oid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK)
-		return NULL;
-
-	/* Check whether a plugin wants to hook function entry/exit */
-	if (FmgrHookIsNeeded(func_oid))
-		return NULL;
-
-	/*
-	 * OK, let's take a look at the function's pg_proc entry.
-	 */
-	func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(func_oid));
-	if (!HeapTupleIsValid(func_tuple))
-		elog(ERROR, "cache lookup failed for function %u", func_oid);
-	funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
-
 	/*
 	 * Forget it if the function is not SQL-language or has other showstopper
 	 * properties.  In particular it mustn't be declared STRICT, since we
@@ -5262,61 +5195,34 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 		funcform->prorettype == VOIDOID ||
 		funcform->prosecdef ||
 		!funcform->proretset ||
-		list_length(fexpr->args) != funcform->pronargs ||
-		!heap_attisnull(func_tuple, Anum_pg_proc_proconfig, NULL))
+		list_length(fexpr->args) != funcform->pronargs)
 	{
-		ReleaseSysCache(func_tuple);
 		return NULL;
 	}
 
-	/*
-	 * Make a temporary memory context, so that we don't leak all the stuff
-	 * that parsing might create.
-	 */
-	mycxt = AllocSetContextCreate(CurrentMemoryContext,
-								  "inline_set_returning_function",
-								  ALLOCSET_DEFAULT_SIZES);
-	oldcxt = MemoryContextSwitchTo(mycxt);
-
-	/* Fetch the function body */
-	tmp = SysCacheGetAttrNotNull(PROCOID, func_tuple, Anum_pg_proc_prosrc);
-	src = TextDatumGetCString(tmp);
-
-	/*
-	 * Setup error traceback support for ereport().  This is so that we can
-	 * finger the function that bad information came from.
-	 */
-	callback_arg.proname = NameStr(funcform->proname);
-	callback_arg.prosrc = src;
-
-	sqlerrcontext.callback = sql_inline_error_callback;
-	sqlerrcontext.arg = &callback_arg;
-	sqlerrcontext.previous = error_context_stack;
-	error_context_stack = &sqlerrcontext;
-
 	/* If we have prosqlbody, pay attention to that not prosrc */
-	tmp = SysCacheGetAttr(PROCOID,
-						  func_tuple,
-						  Anum_pg_proc_prosqlbody,
-						  &isNull);
+	sqlbody = SysCacheGetAttr(PROCOID,
+							  func_tuple,
+							  Anum_pg_proc_prosqlbody,
+							  &isNull);
 	if (!isNull)
 	{
 		Node	   *n;
 
-		n = stringToNode(TextDatumGetCString(tmp));
+		n = stringToNode(TextDatumGetCString(sqlbody));
 		if (IsA(n, List))
 			querytree_list = linitial_node(List, castNode(List, n));
 		else
 			querytree_list = list_make1(n);
 		if (list_length(querytree_list) != 1)
-			goto fail;
+			return NULL;
 		querytree = linitial(querytree_list);
 
 		/* Acquire necessary locks, then apply rewriter. */
 		AcquireRewriteLocks(querytree, true, false);
 		querytree_list = pg_rewrite_query(querytree);
 		if (list_length(querytree_list) != 1)
-			goto fail;
+			return NULL;
 		querytree = linitial(querytree_list);
 	}
 	else
@@ -5337,14 +5243,14 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 		 */
 		raw_parsetree_list = pg_parse_query(src);
 		if (list_length(raw_parsetree_list) != 1)
-			goto fail;
+			return NULL;
 
 		querytree_list = pg_analyze_and_rewrite_withcb(linitial(raw_parsetree_list),
 													   src,
 													   (ParserSetupHook) sql_fn_parser_setup,
 													   pinfo, NULL);
 		if (list_length(querytree_list) != 1)
-			goto fail;
+			return NULL;
 		querytree = linitial(querytree_list);
 	}
 
@@ -5369,7 +5275,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 */
 	if (!IsA(querytree, Query) ||
 		querytree->commandType != CMD_SELECT)
-		goto fail;
+		return NULL;
 
 	/*
 	 * Make sure the function (still) returns what it's declared to.  This
@@ -5391,7 +5297,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 		(functypclass == TYPEFUNC_COMPOSITE ||
 		 functypclass == TYPEFUNC_COMPOSITE_DOMAIN ||
 		 functypclass == TYPEFUNC_RECORD))
-		goto fail;				/* reject not-whole-tuple-result cases */
+		return NULL;			/* reject not-whole-tuple-result cases */
 
 	/*
 	 * check_sql_fn_retval might've inserted a projection step, but that's
@@ -5399,6 +5305,141 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 */
 	querytree = linitial_node(Query, querytree_list);
 
+	return querytree;
+}
+
+/*
+ * inline_set_returning_function
+ *		Attempt to "inline" an SQL set-returning function in the FROM clause.
+ *
+ * "rte" is an RTE_FUNCTION rangetable entry.  If it represents a call of a
+ * set-returning SQL function that can safely be inlined, expand the function
+ * and return the substitute Query structure.  Otherwise, return NULL.
+ *
+ * We assume that the RTE's expression has already been put through
+ * eval_const_expressions(), which among other things will take care of
+ * default arguments and named-argument notation.
+ *
+ * This has a good deal of similarity to inline_function(), but that's
+ * for the non-set-returning case, and there are enough differences to
+ * justify separate functions.
+ *
+ * It allocates its own temporary MemoryContext for the parsing, then copies
+ * the result into the caller's context.
+ */
+Query *
+inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
+{
+	RangeTblFunction *rtfunc;
+	FuncExpr   *fexpr;
+	Oid			func_oid;
+	HeapTuple	func_tuple;
+	Form_pg_proc funcform;
+	Datum		tmp;
+	char	   *src;
+	inline_error_callback_arg callback_arg;
+	ErrorContextCallback sqlerrcontext;
+	MemoryContext oldcxt;
+	MemoryContext mycxt;
+	Query	   *querytree;
+
+	Assert(rte->rtekind == RTE_FUNCTION);
+
+	/*
+	 * It doesn't make a lot of sense for a SRF to refer to itself in its own
+	 * FROM clause, since that must cause infinite recursion at runtime. It
+	 * will cause this code to recurse too, so check for stack overflow.
+	 * (There's no need to do more.)
+	 */
+	check_stack_depth();
+
+	/* Fail if the RTE has ORDINALITY - we don't implement that here. */
+	if (rte->funcordinality)
+		return NULL;
+
+	/* Fail if RTE isn't a single, simple FuncExpr */
+	if (list_length(rte->functions) != 1)
+		return NULL;
+	rtfunc = (RangeTblFunction *) linitial(rte->functions);
+
+	if (!IsA(rtfunc->funcexpr, FuncExpr))
+		return NULL;
+	fexpr = (FuncExpr *) rtfunc->funcexpr;
+
+	func_oid = fexpr->funcid;
+
+	/*
+	 * Refuse to inline if the arguments contain any volatile functions or
+	 * sub-selects.  Volatile functions are rejected because inlining may
+	 * result in the arguments being evaluated multiple times, risking a
+	 * change in behavior.  Sub-selects are rejected partly for implementation
+	 * reasons (pushing them down another level might change their behavior)
+	 * and partly because they're likely to be expensive and so multiple
+	 * evaluation would be bad.
+	 */
+	if (contain_volatile_functions((Node *) fexpr->args) ||
+		contain_subplans((Node *) fexpr->args))
+		return NULL;
+
+	/* Check permission to call function (fail later, if not) */
+	if (object_aclcheck(ProcedureRelationId, func_oid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK)
+		return NULL;
+
+	/* Check whether a plugin wants to hook function entry/exit */
+	if (FmgrHookIsNeeded(func_oid))
+		return NULL;
+
+	/*
+	 * OK, let's take a look at the function's pg_proc entry.
+	 */
+	func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(func_oid));
+	if (!HeapTupleIsValid(func_tuple))
+		elog(ERROR, "cache lookup failed for function %u", func_oid);
+	funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
+
+	/*
+	 * Make a temporary memory context, so that we don't leak all the stuff
+	 * that parsing might create.
+	 */
+	mycxt = AllocSetContextCreate(CurrentMemoryContext,
+								  "inline_set_returning_function",
+								  ALLOCSET_DEFAULT_SIZES);
+	oldcxt = MemoryContextSwitchTo(mycxt);
+
+	/* Fetch the function body */
+	tmp = SysCacheGetAttrNotNull(PROCOID, func_tuple, Anum_pg_proc_prosrc);
+	src = TextDatumGetCString(tmp);
+
+	/*
+	 * Setup error traceback support for ereport().  This is so that we can
+	 * finger the function that bad information came from.
+	 */
+	callback_arg.proname = NameStr(funcform->proname);
+	callback_arg.prosrc = src;
+
+	sqlerrcontext.callback = sql_inline_error_callback;
+	sqlerrcontext.arg = &callback_arg;
+	sqlerrcontext.previous = error_context_stack;
+	error_context_stack = &sqlerrcontext;
+
+	/*
+	 * If the function SETs configuration parameters, inlining would cause us
+	 * to skip those changes.
+	 */
+	if (!heap_attisnull(func_tuple, Anum_pg_proc_proconfig, NULL))
+		goto fail;
+
+	querytree = inline_sql_set_returning_function(root, rte, rtfunc, fexpr,
+												  func_oid, func_tuple, funcform,
+												  src);
+
+	if (!querytree)
+		goto fail;
+
+	/* Only SELECTs are permitted */
+	Assert(IsA(querytree, Query));
+	Assert(querytree->commandType == CMD_SELECT);
+
 	/*
 	 * Looks good --- substitute parameters into the query.
 	 */
-- 
2.45.0