Subject: Re: Inline non-SQL SRFs using SupportRequestSimplify

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 2 Jul 2024 15:59:45 +0300

On 28/06/2024 01:01, Paul Jungwirth wrote:
> If it seems good to let people use SupportRequestSimplify to make their SRFs be inlineable, I'm
> happy to add tests and docs. We should really document the idea of inlined functions in general, so
> I'll do that too.
>
> Another approach I considered is using a separate support request, e.g. SupportRequestInlineSRF, and
> just calling it from inline_set_returning_function. I didn't like having two support requests that
> did almost exactly the same thing. OTOH my current approach means you'll get an error if you do this:
> 
> ```
> postgres=# select temporal_semijoin('employees', 'id', 'valid_at', 'positions', 'employee_id',
> 'valid_at');
> ERROR:  unrecognized node type: 66
> ```
> 
> I'll look into ways to fix that.

If the support function returns a Query, we end up having a FuncExpr 
with a Query in the tree. A Query isnt an Expr, which is why you get 
that error, and it seems like a recipe for confusion in general. Perhaps 
returning a SubLink would be better.

I think we should actually add an assertion after the call to the 
SupportRequestSimplify support function, to check that it returned an 
Expr node.

+1 to the general feature of letting SRFs be simplified by the support 
function.

> I think SupportRequestSimplify is a really cool feature. It is nearly like having macros.
> I'm dreaming about other ways I can (ab)use it.

:-D

-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Tom Lane <[email-masked]>
Date: Fri, 26 Jul 2024 14:58:54 -0400

>> I'll look into ways to fix that.

I like this idea, but I like exactly nothing about this implementation.
The right thing is to have a separate SupportRequestInlineSRF request
that is called directly by inline_set_returning_function.  It might be
"almost the same thing" as SupportRequestSimplify, but "almost" only
counts in horseshoes and hand grenades.  In particular, returning a
Query node is simply broken for SupportRequestSimplify (as your
example demonstrates), whereas it's the only correct result for
SupportRequestInlineSRF.

You could imagine keeping it to one support request by adding a
boolean field to the request struct to show which behavior is wanted,
but I think the principal result of that would be to break extensions
that weren't expecting such calls.  The defined mechanism for
extending the SupportRequest protocol is to add new support request
codes, not to whack around the APIs of existing ones.

> SupportRequestSimplify support function, to check that it returned an 

Um ... IsA(node, Expr) isn't going to work, and I'm not sure that
it'd be useful to try to enumerate the set of Expr subtypes that
should be allowed there.  But possibly it'd be worth asserting that
it's not a Query, just in case anyone gets confused about the
difference between SupportRequestSimplify and SupportRequestInlineSRF.

It would be good to have an in-core test case for this request type,
but I don't really see any built-in SRFs for which expansion as a
sub-SELECT would be an improvement.

			regards, tom lane

---

From: Paul Jungwirth <[email-masked]>
Date: Fri, 30 Aug 2024 09:26:56 -0700

On 7/26/24 11:58, Tom Lane wrote:
 > that is called directly by inline_set_returning_function.

Here are new patches using a new SupportRequestInlineSRF request type. They include patches and 
documentation.

The patches handle this:

    SELECT * FROM srf();

but not this:

    SELECT srf();

In the latter case, Postgres always calls the function in "materialized mode" and gets the whole 
result up front, so inline_set_returning_function is never called, even for SQL functions.

For tests I added a `foo_from_bar(colname, tablename, filter)` PL/pgSQL function that does `SELECT 
$colname FROM $tablename [WHERE $colname = $filter]`, then the support function generates the same 
SQL and turns it into a Query node. This matches how I want to use the feature for my 
temporal_semijoin etc functions. If you give a non-NULL filter, you get a Query with a Var node, so 
we are testing something that isn't purely Const.

The SupportRequestSimplify type has some comments about supporting operators, but I don't think you 
can have a set-returning operator, so I didn't repeat those comments for this new type.

I split things up into three patch files because I couldn't get git to gracefully handle shifting a 
large block of code into an if statement. The first two patches have no changes except that 
indentation (and initializing one variable to NULL). They aren't meant to be committed separately.

Rebased to a83a944e9f.

Yours,

-- 
Paul              ~{:-)
[email-masked]

From 591de3a06ec97cf1bff10d603946f0bac176b9d9 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email-masked]>
Date: Thu, 29 Aug 2024 18:17:55 -0700
Subject: [PATCH v2 1/3] Add indented section

---
 src/backend/optimizer/util/clauses.c | 74 +++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index b4e085e9d4b..ef8282ded49 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5069,7 +5069,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	TupleDesc	rettupdesc;
 	List	   *raw_parsetree_list;
 	List	   *querytree_list;
-	Query	   *querytree;
+	Query	   *querytree = NULL;
 
 	Assert(rte->rtekind == RTE_FUNCTION);
 
@@ -5235,6 +5235,78 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 			goto fail;
 		querytree = linitial(querytree_list);
 	}
+	if (!querytree)
+	{
+		/* 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 = (void *) &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);
+		if (!isNull)
+		{
+			Node	   *n;
+
+			n = stringToNode(TextDatumGetCString(tmp));
+			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;
+			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;
+			querytree = linitial(querytree_list);
+		}
+		else
+		{
+			/*
+			 * Set up to handle parameters while parsing the function body.  We
+			 * can use the FuncExpr just created as the input for
+			 * prepare_sql_fn_parse_info.
+			 */
+			pinfo = prepare_sql_fn_parse_info(func_tuple,
+											  (Node *) fexpr,
+											  fexpr->inputcollid);
+
+			/*
+			 * Parse, analyze, and rewrite (unlike inline_function(), we can't
+			 * skip rewriting here).  We can fail as soon as we find more than one
+			 * query, though.
+			 */
+			raw_parsetree_list = pg_parse_query(src);
+			if (list_length(raw_parsetree_list) != 1)
+				goto fail;
+
+			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;
+			querytree = linitial(querytree_list);
+		}
+	}
 
 	/*
 	 * Also resolve the actual function result tupdesc, if composite.  If we
-- 
2.42.0

From 7a9ee15d17bee6a6743bf18e7428e6594e17be35 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email-masked]>
Date: Thu, 29 Aug 2024 18:18:20 -0700
Subject: [PATCH v2 2/3] Remove unindented section

---
 src/backend/optimizer/util/clauses.c | 69 ----------------------------
 1 file changed, 69 deletions(-)

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index ef8282ded49..d2d8f7eb0c0 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5166,75 +5166,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 								  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 = (void *) &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);
-	if (!isNull)
-	{
-		Node	   *n;
-
-		n = stringToNode(TextDatumGetCString(tmp));
-		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;
-		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;
-		querytree = linitial(querytree_list);
-	}
-	else
-	{
-		/*
-		 * Set up to handle parameters while parsing the function body.  We
-		 * can use the FuncExpr just created as the input for
-		 * prepare_sql_fn_parse_info.
-		 */
-		pinfo = prepare_sql_fn_parse_info(func_tuple,
-										  (Node *) fexpr,
-										  fexpr->inputcollid);
-
-		/*
-		 * Parse, analyze, and rewrite (unlike inline_function(), we can't
-		 * skip rewriting here).  We can fail as soon as we find more than one
-		 * query, though.
-		 */
-		raw_parsetree_list = pg_parse_query(src);
-		if (list_length(raw_parsetree_list) != 1)
-			goto fail;
-
-		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;
-		querytree = linitial(querytree_list);
-	}
 	if (!querytree)
 	{
 		/* Fetch the function body */
-- 
2.42.0

From 4ed50cad16901c5b7b3447bad7105de11a81773a Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email-masked]>
Date: Thu, 29 Aug 2024 18:20:47 -0700
Subject: [PATCH v2 3/3] Inline non-SQL SRFs using SupportRequestInlineSRF

By inlining functions written in PL/pgSQL (or other languages), we let
users compose dynamic queries without paying the overhead of function
invocations. Formerly this was only possible for SQL functions.

If the support request returns a Query node, then
inline_set_returning_function will use it in place of the FuncExpr.
The Query node can reference the original function's arguments as Var
nodes.

We only try to inline SRFs (even SQL SRFs) when they are used as RTEs,
so inlining happens for `SELECT * FROM foo()` but not for `SELECT
foo()`.
---
 doc/src/sgml/xfunc.sgml                      |  14 ++
 src/backend/optimizer/util/clauses.c         |  63 +++++-
 src/include/nodes/supportnodes.h             |  22 ++
 src/test/regress/expected/misc_functions.out | 212 +++++++++++++++++++
 src/test/regress/regress.c                   |  91 ++++++++
 src/test/regress/sql/misc_functions.sql      |  87 ++++++++
 6 files changed, 487 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 9bc23a9a938..311a87b14a5 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -4076,6 +4076,20 @@ supportfn(internal) returns internal
     expression and an actual execution of the target function.
    </para>
 
+   <para>
+    Similarly, a <link linkend="queries-tablefunction">set-returning function</link>
+    can implement <literal>SupportRequestInlineSRF</literal> to return a
+    <literal>Query</literal> node, which the planner will try to inline into
+    the outer query, just as <productname>PostgreSQL</productname> inlines
+    SQL functions.  Normallly only SQL functions can be inlined, but this support
+    request allows a function in <link linkend="plpgsql">PL/pgSQL</link>
+    or another language to build a dynamic SQL query and have it inlined too.
+    It is the responsibility of the support function to return
+    a node that matches the parent function's implementation. One way to do this is
+    to build a SQL string then parse it with <literal>pg_parse_query</literal>.
+    See <literal>regress.c</literal> for an example.
+   </para>
+
    <para>
     For target functions that return <type>boolean</type>, it is often useful to estimate
     the fraction of rows that will be selected by a <literal>WHERE</literal> clause using that
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index d2d8f7eb0c0..9ee5831ed0e 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5134,7 +5134,8 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
 
 	/*
-	 * Forget it if the function is not SQL-language or has other showstopper
+	 * Forget it unless the function is SQL-language or provides a support function.
+	 * Also check for other showstopper
 	 * properties.  In particular it mustn't be declared STRICT, since we
 	 * couldn't enforce that.  It also mustn't be VOLATILE, because that is
 	 * supposed to cause it to be executed with its own snapshot, rather than
@@ -5143,7 +5144,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 * of the function's last SELECT, which should not happen in that case.
 	 * (Rechecking prokind, proretset, and pronargs is just paranoia.)
 	 */
-	if (funcform->prolang != SQLlanguageId ||
+	if ((funcform->prolang != SQLlanguageId && !funcform->prosupport) ||
 		funcform->prokind != PROKIND_FUNCTION ||
 		funcform->proisstrict ||
 		funcform->provolatile == PROVOLATILE_VOLATILE ||
@@ -5166,6 +5167,64 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 								  ALLOCSET_DEFAULT_SIZES);
 	oldcxt = MemoryContextSwitchTo(mycxt);
 
+	/*
+	 * If the function has a SupportRequestInlineSRF support function,
+	 * see if it can produce a Query node we can inline.
+	 * This can be mutually-exclusive with SQL functions.
+	 * Those are inlineable already, so it doesn't make sense to
+	 * attach such a support request to them.
+	 */
+	if (funcform->prolang != SQLlanguageId && funcform->prosupport) {
+		SupportRequestInlineSRF req;
+		FuncExpr	dummy_fexpr;
+		Node	   *newnode;
+
+		/*
+		 * We don't need the whole sqlerrcontext,
+		 * but make sure we restore it correctly
+		 * if we goto fail.
+		 */
+		sqlerrcontext.previous = error_context_stack;
+
+		/*
+		 * Build a SupportRequestInlineSRF node to pass to the support
+		 * function, pointing to a dummy FuncExpr node containing the
+		 * simplified arg list.  We use this approach to present a uniform
+		 * interface to the support function regardless of how the target
+		 * function is actually being invoked.
+		 */
+
+		dummy_fexpr.xpr.type = T_FuncExpr;
+		dummy_fexpr.funcid = func_oid;
+		dummy_fexpr.funcresulttype = funcform->prorettype;
+		dummy_fexpr.funcretset = funcform->proretset;
+		dummy_fexpr.funcvariadic = OidIsValid(funcform->provariadic);
+		dummy_fexpr.funcformat = COERCE_EXPLICIT_CALL;
+		dummy_fexpr.funccollid = InvalidOid;
+		dummy_fexpr.inputcollid = InvalidOid;
+		dummy_fexpr.args = fexpr->args;
+		dummy_fexpr.location = -1;
+
+		req.type = T_SupportRequestInlineSRF;
+		req.fcall = &dummy_fexpr;
+
+		newnode = (Node *)
+			DatumGetPointer(OidFunctionCall1(funcform->prosupport,
+											 PointerGetDatum(&req)));
+
+		if (!newnode)
+			goto fail;
+
+		if (!IsA(newnode, Query))
+			elog(ERROR,
+				 "Got unexpected node type %d from SupportRequestInlineSRF for function %s",
+				 newnode->type, NameStr(funcform->proname));
+
+		querytree = (Query *) newnode;
+		querytree_list = list_make1(querytree);
+	}
+
+
 	if (!querytree)
 	{
 		/* Fetch the function body */
diff --git a/src/include/nodes/supportnodes.h b/src/include/nodes/supportnodes.h
index 5f7bcde891c..bc0b5cccaf3 100644
--- a/src/include/nodes/supportnodes.h
+++ b/src/include/nodes/supportnodes.h
@@ -69,6 +69,28 @@ typedef struct SupportRequestSimplify
 	FuncExpr   *fcall;			/* Function call to be simplified */
 } SupportRequestSimplify;
 
+/*
+ * The InlineSRF request allows the support function to perform plan-time
+ * simplification of a call to its target set-returning function. For
+ * example a plpgsql function could build a dynamic SQL query and execute it.
+ * Normally only SQL functions can be inlined, but with this support function
+ * the dynamic query can be inlined as well.
+ *
+ * "fcall" will be a FuncExpr invoking the support function's target
+ * function.
+ *
+ * The result should be a semantically-equivalent transformed node tree,
+ * or NULL if no simplification could be performed.  Do *not* return or
+ * modify *fcall, as it isn't really a separately allocated Node.  But
+ * it's okay to use fcall->args, or parts of it, in the result tree.
+ */
+typedef struct SupportRequestInlineSRF
+{
+	NodeTag		type;
+
+	FuncExpr   *fcall;			/* Function call to be simplified */
+} SupportRequestInlineSRF;
+
 /*
  * The Selectivity request allows the support function to provide a
  * selectivity estimate for a function appearing at top level of a WHERE
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 35fb72f302b..c5bac6d43ea 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -595,6 +595,11 @@ CREATE FUNCTION test_support_func(internal)
     RETURNS internal
     AS :'regresslib', 'test_support_func'
     LANGUAGE C STRICT;
+-- With a support function that inlines SRFs
+CREATE FUNCTION test_inline_srf_support_func(internal)
+    RETURNS internal
+    AS :'regresslib', 'test_inline_srf_support_func'
+    LANGUAGE C STRICT;
 ALTER FUNCTION my_int_eq(int, int) SUPPORT test_support_func;
 EXPLAIN (COSTS OFF)
 SELECT * FROM tenk1 a JOIN tenk1 b ON a.unique1 = b.unique1
@@ -706,6 +711,213 @@ false, true, false, true);
 -- the support function.
 SELECT * FROM generate_series(TIMESTAMPTZ '2024-02-01', TIMESTAMPTZ '2024-03-01', INTERVAL '0 day') g(s);
 ERROR:  step size cannot equal zero
+--
+-- Test inlining PL/pgSQL functions
+--
+-- RETURNS SETOF TEXT:
+CREATE OR REPLACE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS SETOF TEXT
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE LEAKPROOF;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+   foo_from_bar    
+-------------------
+ doh!
+ hi de ho neighbor
+(2 rows)
+
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+ foo_from_bar 
+--------------
+ doh!
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+                              QUERY PLAN                              
+----------------------------------------------------------------------
+ Function Scan on foo_from_bar  (cost=0.25..10.25 rows=1000 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+                              QUERY PLAN                              
+----------------------------------------------------------------------
+ Function Scan on foo_from_bar  (cost=0.25..10.25 rows=1000 width=32)
+(1 row)
+
+ALTER FUNCTION foo_from_bar(TEXT, TEXT, TEXT) SUPPORT test_inline_srf_support_func;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+   foo_from_bar    
+-------------------
+ doh!
+ hi de ho neighbor
+(2 rows)
+
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+ foo_from_bar 
+--------------
+ doh!
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Seq Scan on text_tbl  (cost=0.00..1.02 rows=2 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Seq Scan on text_tbl  (cost=0.00..1.02 rows=1 width=32)
+   Filter: (f1 = 'doh!'::text)
+(2 rows)
+
+DROP FUNCTION foo_from_bar;
+-- RETURNS SETOF RECORD:
+CREATE OR REPLACE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS SETOF RECORD
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE LEAKPROOF;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+        foo        
+-------------------
+ doh!
+ hi de ho neighbor
+(2 rows)
+
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+ foo  
+------
+ doh!
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+                                QUERY PLAN                                
+--------------------------------------------------------------------------
+ Function Scan on foo_from_bar bar  (cost=0.25..10.25 rows=1000 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+                                QUERY PLAN                                
+--------------------------------------------------------------------------
+ Function Scan on foo_from_bar bar  (cost=0.25..10.25 rows=1000 width=32)
+(1 row)
+
+ALTER FUNCTION foo_from_bar(TEXT, TEXT, TEXT) SUPPORT test_inline_srf_support_func;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+        foo        
+-------------------
+ doh!
+ hi de ho neighbor
+(2 rows)
+
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+ foo  
+------
+ doh!
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Seq Scan on text_tbl  (cost=0.00..1.02 rows=2 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Seq Scan on text_tbl  (cost=0.00..1.02 rows=1 width=32)
+   Filter: (f1 = 'doh!'::text)
+(2 rows)
+
+DROP FUNCTION foo_from_bar;
+-- RETURNS TABLE:
+CREATE OR REPLACE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS TABLE(foo TEXT)
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE LEAKPROOF;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+        foo        
+-------------------
+ doh!
+ hi de ho neighbor
+(2 rows)
+
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+ foo  
+------
+ doh!
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+                              QUERY PLAN                              
+----------------------------------------------------------------------
+ Function Scan on foo_from_bar  (cost=0.25..10.25 rows=1000 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+                              QUERY PLAN                              
+----------------------------------------------------------------------
+ Function Scan on foo_from_bar  (cost=0.25..10.25 rows=1000 width=32)
+(1 row)
+
+ALTER FUNCTION foo_from_bar(TEXT, TEXT, TEXT) SUPPORT test_inline_srf_support_func;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+        foo        
+-------------------
+ doh!
+ hi de ho neighbor
+(2 rows)
+
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+ foo  
+------
+ doh!
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Seq Scan on text_tbl  (cost=0.00..1.02 rows=2 width=32)
+(1 row)
+
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Seq Scan on text_tbl  (cost=0.00..1.02 rows=1 width=32)
+   Filter: (f1 = 'doh!'::text)
+(2 rows)
+
+DROP FUNCTION foo_from_bar;
 -- Test functions for control data
 SELECT count(*) > 0 AS ok FROM pg_control_checkpoint();
  ok 
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 14aad5a0c6e..9f97ad6ad45 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -29,6 +29,7 @@
 #include "commands/sequence.h"
 #include "commands/trigger.h"
 #include "executor/executor.h"
+#include "executor/functions.h"
 #include "executor/spi.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
@@ -39,11 +40,13 @@
 #include "parser/parse_coerce.h"
 #include "port/atomics.h"
 #include "storage/spin.h"
+#include "tcop/tcopprot.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/geo_decls.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
+#include "utils/syscache.h"
 #include "utils/typcache.h"
 
 #define EXPECT_TRUE(expr)	\
@@ -989,6 +992,94 @@ test_support_func(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(ret);
 }
 
+PG_FUNCTION_INFO_V1(test_inline_srf_support_func);
+Datum
+test_inline_srf_support_func(PG_FUNCTION_ARGS)
+{
+	Node	   *rawreq = (Node *) PG_GETARG_POINTER(0);
+
+	if (IsA(rawreq, SupportRequestInlineSRF))
+	{
+		/*
+		 * Assume that the target is foo_from_bar; that's safe as long
+		 * as we don't attach this to any other set-returning function.
+		 */
+		SupportRequestInlineSRF *req = (SupportRequestInlineSRF *) rawreq;
+		StringInfoData sql;
+		FuncExpr *expr = req->fcall;
+		Node *node;
+		Const *c;
+		char *colname;
+		char *tablename;
+		HeapTuple func_tuple;
+		SQLFunctionParseInfoPtr pinfo;
+		List *raw_parsetree_list;
+		List *querytree_list;
+		Query *querytree;
+
+		if (list_length(expr->args) != 3)
+			ereport(ERROR, (errmsg("test_inline_srf_support_func called with %d args but expected 3", list_length(expr->args))));
+
+		/* Get colname */
+		node = linitial(expr->args);
+		if (!IsA(node, Const))
+			ereport(ERROR, (errmsg("test_inline_srf_support_func called with non-Const parameters")));
+		c = (Const *) node;
+		if (c->consttype != TEXTOID)
+			ereport(ERROR, (errmsg("test_inline_srf_support_func called with non-TEXT parameters")));
+		colname = TextDatumGetCString(c->constvalue);
+
+		/* Get tablename */
+		node = lsecond(expr->args);
+		if (!IsA(node, Const))
+			ereport(ERROR, (errmsg("test_inline_srf_support_func called with non-Const parameters")));
+		c = (Const *) node;
+		if (c->consttype != TEXTOID)
+			ereport(ERROR, (errmsg("test_inline_srf_support_func called with non-TEXT parameters")));
+		tablename = TextDatumGetCString(c->constvalue);
+
+		initStringInfo(&sql);
+		appendStringInfo(&sql, "SELECT %s::text FROM %s", quote_identifier(colname), quote_identifier(tablename));
+
+		/* Get filter if present */
+		node = lthird(expr->args);
+		if (!(IsA(node, Const) && ((Const *) node)->constisnull))
+		{
+			appendStringInfo(&sql, " WHERE %s::text = $3", quote_identifier(colname));
+		}
+
+		func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(expr->funcid));
+		if (!HeapTupleIsValid(func_tuple))
+			elog(ERROR, "cache lookup failed for function %u", expr->funcid);
+
+		pinfo = prepare_sql_fn_parse_info(func_tuple,
+										  (Node *) expr,
+										  expr->inputcollid);
+
+		ReleaseSysCache(func_tuple);
+
+		raw_parsetree_list = pg_parse_query(sql.data);
+		if (list_length(raw_parsetree_list) != 1)
+			elog(ERROR, "test_inline_srf_support_func parsed to more than one node");
+
+		querytree_list = pg_analyze_and_rewrite_withcb(
+				linitial(raw_parsetree_list),
+				sql.data,
+				(ParserSetupHook) sql_fn_parser_setup,
+				pinfo, NULL);
+		if (list_length(querytree_list) != 1)
+			elog(ERROR, "test_inline_srf_support_func rewrote to more than one node");
+
+		querytree = linitial(querytree_list);
+		if (!IsA(querytree, Query))
+			ereport(ERROR, (errmsg("test_inline_srf_support_func didn't parse to a Query"),
+							errdetail("Got this instead: %s", nodeToString(querytree))));
+		PG_RETURN_POINTER(querytree);
+	}
+
+	PG_RETURN_POINTER(NULL);
+}
+
 PG_FUNCTION_INFO_V1(test_opclass_options_func);
 Datum
 test_opclass_options_func(PG_FUNCTION_ARGS)
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index e570783453c..bf692edd0b7 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -247,6 +247,12 @@ CREATE FUNCTION test_support_func(internal)
     AS :'regresslib', 'test_support_func'
     LANGUAGE C STRICT;
 
+-- With a support function that inlines SRFs
+CREATE FUNCTION test_inline_srf_support_func(internal)
+    RETURNS internal
+    AS :'regresslib', 'test_inline_srf_support_func'
+    LANGUAGE C STRICT;
+
 ALTER FUNCTION my_int_eq(int, int) SUPPORT test_support_func;
 
 EXPLAIN (COSTS OFF)
@@ -265,6 +271,8 @@ SELECT * FROM tenk1 a JOIN my_gen_series(1,1000) g ON a.unique1 = g;
 EXPLAIN (COSTS OFF)
 SELECT * FROM tenk1 a JOIN my_gen_series(1,10) g ON a.unique1 = g;
 
+
+
 --
 -- Test the SupportRequestRows support function for generate_series_timestamp()
 --
@@ -310,6 +318,85 @@ false, true, false, true);
 -- the support function.
 SELECT * FROM generate_series(TIMESTAMPTZ '2024-02-01', TIMESTAMPTZ '2024-03-01', INTERVAL '0 day') g(s);
 
+--
+-- Test inlining PL/pgSQL functions
+--
+-- RETURNS SETOF TEXT:
+CREATE OR REPLACE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS SETOF TEXT
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE LEAKPROOF;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+ALTER FUNCTION foo_from_bar(TEXT, TEXT, TEXT) SUPPORT test_inline_srf_support_func;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+DROP FUNCTION foo_from_bar;
+-- RETURNS SETOF RECORD:
+CREATE OR REPLACE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS SETOF RECORD
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE LEAKPROOF;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+ALTER FUNCTION foo_from_bar(TEXT, TEXT, TEXT) SUPPORT test_inline_srf_support_func;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL) AS bar(foo TEXT);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!') AS bar(foo TEXT);
+DROP FUNCTION foo_from_bar;
+-- RETURNS TABLE:
+CREATE OR REPLACE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS TABLE(foo TEXT)
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE LEAKPROOF;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+ALTER FUNCTION foo_from_bar(TEXT, TEXT, TEXT) SUPPORT test_inline_srf_support_func;
+SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', NULL);
+EXPLAIN SELECT * FROM foo_from_bar('f1', 'text_tbl', 'doh!');
+DROP FUNCTION foo_from_bar;
+
 -- Test functions for control data
 SELECT count(*) > 0 AS ok FROM pg_control_checkpoint();
 SELECT count(*) > 0 AS ok FROM pg_control_init();
-- 
2.42.0

---

From: Tom Lane <[email-masked]>
Date: Tue, 03 Sep 2024 12:42:01 -0400

> Here are new patches using a new SupportRequestInlineSRF request type. They include patches and 

I took a look through this.  I feel like we're still some way away
from having something committable.  I've got two main complaint
areas:

1. It doesn't seem like integrating this into
inline_set_returning_function was the right thing after all, or
maybe just the way you did it isn't right.  That function is pretty
opinionated about what it is doing, and a lot of what it is doing
doesn't seem appropriate for a support-function-driven substitution.
As an example, it rejects WITH ORDINALITY, but who's to say that a
support function couldn't handle that?  More generally, I'm not sure
if it's appropriate to make any tests on the function's properties,
rather than assuming the support function knows what it's doing.
I see you already hacked up the test on prolang, but the others in
the same if-clause seem equally dubious from here.  I'm also unsure
whether it's our business to reject volatile functions or subplans
in the function arguments.  (Maybe it is, but not sure.)  There is
also stuff towards the bottom of the function, particularly
check_sql_fn_retval and parameter substitution, that I do not think
makes sense to apply to a non-SQL-language function; but if I'm
reading this right you run all that code on the support function's
result.

It does make sense to require there to be just one RangeTblFunction in
the RTE, since it's not at all clear how we could combine the results
if there's more than one.  But I wonder if we should just pass the RTE
node to the support function, and let it make its own decision about
rte->funcordinality.  Or if that seems like a bad idea, pass the
RangeTblFunction node.  I think it's essential to do one of those
things rather than fake up a FuncExpr, because a support function for
a function returning RECORD would likely need access to the column
definition list to figure out what to do.

I notice that in the case of non-SRF function inlining, we handle
support-function calling in a totally separate function
(simplify_function) rather than try to integrate it into the
code that does SQL function inlining (inline_function).  Maybe
a similar approach should be adopted here.  We could have a
wrapper function that implements the parts worth sharing, such
as looking up the target function's pg_proc entry and doing
the permissions check.  Or perhaps put that stuff into the sole
caller, preprocess_function_rtes.

If we do keep this in inline_set_returning_function, we need to
pay more than zero attention to updating that function's header
comment.

2. The documentation needs to be a great deal more explicit
about what the function is supposed to return.  It needs to
be a SELECT Query node that has been through parse analysis
and rewriting.  I don't think pointing to a regression test
function is adequate, or even appropriate.  The test function
is a pretty bad example as-is, too.  It aggressively disregards
the API recommendation in supportnodes.h:

 * Support functions must return a NULL pointer, not fail, if they do not
 * recognize the request node type or cannot handle the given case; this
 * allows for future extensions of the set of request cases.

As a more minor nit, I think SupportRequestInlineSRF should
include "struct PlannerInfo *root", for the same reasons that
SupportRequestSimplify does.

> indentation (and initializing one variable to NULL). They aren't meant to be committed separately.

A hack I've used in the past is to have the main patch just add

+	if (...)
+	{
...
+	}

around the to-be-reindented code, and then apply pgindent as a
separate patch step.  (We used to just leave it to the committer to
run pgindent, but I think nowadays the cfbot will whine at you if you
submit not-pgindented code.)  I think that's easier to review since
the reviewer can mechanically verify the pgindent patch.  This problem
may be moot for this patch once we detangle the support function call
from SQL-function inlining, though.

			regards, tom lane

---

From: Paul Jungwirth <[email-masked]>
Date: Mon, 30 Jun 2025 14:42:08 -0700

On 9/3/24 09:42, Tom Lane wrote:
> maybe just the way you did it isn't right.
 > ...
> about what the function is supposed to return.

Thanks for the review . . . and your patience waiting for an update!

I tried a few refactoring approaches but the nicest seemed to be to keep the shared parts in 
inline_set_returning_function, but have it call out to either inline_sql_set_returning_function or 
inline_set_returning_function_with_support. The first patch just refactors but doesn't yet add 
inline_set_returning_function_with_support, then the second patch adds the new functionality.

The refactor lets us share lots of pre-condition checks, as well as parameter substitution into the 
Query result. In some cases the refactor changes the order of things, but all of those changes 
looked safe to me. I didn't love passing a SysCache HeapTuple into another function, but it does 
make the cleanup a little easier, since now we can always release it in the same place.

The first patch gave me a wacky diff, but I couldn't get git to make something less fragmented. The 
idea is simple though: move part of inline_set_returning_function into 
inline_sql_set_returning_function, and call that instead.

Rebased to 0836683a89.

Yours,

-- 
Paul              ~{:-)
[email-masked]
From 2a0d2bb6cc6fe25cab1065411ed4db17cc394c53 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email-masked]>
Date: Tue, 24 Jun 2025 19:10:15 -0700
Subject: [PATCH v2 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 <[email-masked]>
---
 src/backend/optimizer/util/clauses.c | 212 +++++++++++++++------------
 1 file changed, 122 insertions(+), 90 deletions(-)

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 26a3e050086..a0dc5dfcd61 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5049,29 +5049,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).
+ * It allocates its own temporary MemoryContext for the parsing, then copies
+ * the result into the caller's context.
  *
- * 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)
 {
-	RangeTblFunction *rtfunc;
-	FuncExpr   *fexpr;
-	Oid			func_oid;
-	HeapTuple	func_tuple;
-	Form_pg_proc funcform;
 	char	   *src;
 	Datum		tmp;
 	bool		isNull;
@@ -5088,37 +5080,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)

-	/*
-	 * 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
-	 * row.
-	 */
-	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
@@ -5132,22 +5093,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 		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
@@ -5168,7 +5113,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 		list_length(fexpr->args) != funcform->pronargs ||
 		!heap_attisnull(func_tuple, Anum_pg_proc_proconfig, NULL))
 	{
-		ReleaseSysCache(func_tuple);
 		return NULL;
 	}
 
@@ -5177,7 +5121,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 * that parsing might create.
 	 */
 	mycxt = AllocSetContextCreate(CurrentMemoryContext,
-								  "inline_set_returning_function",
+								  "inline_sql_set_returning_function",
 								  ALLOCSET_DEFAULT_SIZES);
 	oldcxt = MemoryContextSwitchTo(mycxt);
 
@@ -5267,13 +5211,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	else
 		functypclass = get_expr_result_type((Node *) fexpr, NULL, &rettupdesc);
 
-	/*
-	 * The single command must be a plain SELECT.
-	 */
-	if (!IsA(querytree, Query) ||
-		querytree->commandType != CMD_SELECT)
-		goto fail;
-
 	/*
 	 * Make sure the function (still) returns what it's declared to.  This
 	 * will raise an error if wrong, but that's okay since the function would
@@ -5302,13 +5239,6 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 */
 	querytree = linitial_node(Query, querytree_list);
 
-	/*
-	 * Looks good --- substitute parameters into the query.
-	 */
-	querytree = substitute_actual_srf_parameters(querytree,
-												 funcform->pronargs,
-												 fexpr->args);
-
 	/*
 	 * Copy the modified query out of the temporary memory context, and clean
 	 * up.
@@ -5319,7 +5249,112 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 
 	MemoryContextDelete(mycxt);
 	error_context_stack = sqlerrcontext.previous;
-	ReleaseSysCache(func_tuple);
+
+	return querytree;
+
+	/* Here if func is not inlinable: release temp memory and return NULL */
+fail:
+	MemoryContextSwitchTo(oldcxt);
+	MemoryContextDelete(mycxt);
+	error_context_stack = sqlerrcontext.previous;
+
+	return NULL;
+}
+
+/*
+ * 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.
+ */
+Query *
+inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
+{
+	RangeTblFunction *rtfunc;
+	FuncExpr   *fexpr;
+	Oid			func_oid;
+	HeapTuple	func_tuple;
+	Form_pg_proc funcform;
+	Query	   *funcquery;
+
+	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;
+
+	/*
+	 * The function must be declared to return a set, else inlining would
+	 * change the results if the contained SELECT didn't return exactly one
+	 * row.
+	 */
+	if (!fexpr->funcretset)
+		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);
+
+	funcquery = inline_sql_set_returning_function(root, rte, rtfunc, fexpr,
+												  func_oid, func_tuple, funcform);
+
+	if (!funcquery)
+		goto fail;
+
+	/*
+	 * The single command must be a plain SELECT.
+	 */
+	if (!IsA(funcquery, Query) ||
+		funcquery->commandType != CMD_SELECT)
+		goto fail;
+
+	/*
+	 * Looks good --- substitute parameters into the query.
+	 */
+	funcquery = substitute_actual_srf_parameters(funcquery,
+												 funcform->pronargs,
+												 fexpr->args);
 
 	/*
 	 * We don't have to fix collations here because the upper query is already
@@ -5336,18 +5371,15 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 * We must also notice if the inserted query adds a dependency on the
 	 * calling role due to RLS quals.
 	 */
-	if (querytree->hasRowSecurity)
+	if (funcquery->hasRowSecurity)
 		root->glob->dependsOnRole = true;
 
-	return querytree;
+	return funcquery;
 
-	/* Here if func is not inlinable: release temp memory and return NULL */
 fail:
-	MemoryContextSwitchTo(oldcxt);
-	MemoryContextDelete(mycxt);
-	error_context_stack = sqlerrcontext.previous;
 	ReleaseSysCache(func_tuple);
-
 	return NULL;
 }
 
-- 
2.39.5

From c48655640c56055ee5d10fd6feb804a88d7eee29 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email-masked]>
Date: Tue, 24 Jun 2025 21:30:02 -0700
Subject: [PATCH v2 2/2] Add SupportRequestInlineSRF

If a set-returning function has an attached support function that can
handle SupportRequestInlineSRF, then we replace the FuncExpr with a
Query node built by the support function. Then the planner can rewrite
the Query as if it were from a SQL-language function, merging it with
the outer query.

Author: Paul A. Jungwirth <[email-masked]>
---
 doc/src/sgml/xfunc.sgml                      | 101 +++++++++
 src/backend/optimizer/util/clauses.c         |  65 +++++-
 src/include/nodes/supportnodes.h             |  33 +++
 src/test/regress/expected/misc_functions.out | 212 +++++++++++++++++++
 src/test/regress/regress.c                   | 122 +++++++++++
 src/test/regress/sql/misc_functions.sql      |  86 ++++++++
 6 files changed, 614 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 2d81afce8cb..ef41ce49e65 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -4166,6 +4166,107 @@ supportfn(internal) returns internal
     expression and an actual execution of the target function.
    </para>
 
+   <para>
+    Similarly, a <link linkend="queries-tablefunctions">set-returning function</link>
+    can implement <literal>SupportRequestInlineSRF</literal> to return a
+    <literal>Query</literal> node, which the planner will try to inline into
+    the outer query, just as <productname>PostgreSQL</productname> inlines
+    SQL functions.  Normallly only SQL functions can be inlined, but this support
+    request allows a function in <link linkend="plpgsql">PL/pgSQL</link>
+    or another language to build a dynamic SQL query and have it inlined too.
+    The <literal>Query</literal> node must be a <literal>SELECT</literal> query
+    that has gone through parse analysis and rewriting.
+    You may include <literal>Param</literal> nodes referencing the original function's
+    parameters, and <productname>PostgreSQL</productname> will map those appropriately
+    to the arguments passed by the caller.
+    It is the responsibility of the support function to return
+    a node that matches the parent function's implementation.
+    We make no guarantee that <productname>PostgreSQL</productname> will
+    never call the target function in cases that the support function could
+    simplify.  Functions called in <literal>SELECT</literal> are not simplified.
+    Or if the <literal>RangeTblEntry</literal> has more than one
+    <literal>RangeTblFunction</literal> (such as when using
+    <literal>ROWS FROM</literal>), the function will not be simplified.
+    Ensure rigorous equivalence between the simplified expression and an actual
+    execution of the target function.
+   </para>
+
+   <para>
+    One way to implement a <literal>SupportRequestInlineSRF</literal> support function
+    is to build a SQL string then parse it with <literal>pg_parse_query</literal>.
+    The outline of such a function might look like this:
+<programlisting>
+PG_FUNCTION_INFO_V1(my_support_function);
+Datum
+my_support_function(PG_FUNCTION_ARGS)
+{
+    Node                    *rawreq = (Node *) PG_GETARG_POINTER(0);
+    SupportRequestInlineSRF *req
+    RangeTblFunction        *rtfunc;
+    FuncExpr                *expr;
+    Query                   *querytree;
+    StringInfoData           sql;
+    HeapTuple                func_tuple;
+    SQLFunctionParseInfoPtr  pinfo;
+    List                    *raw_parsetree_list;
+
+    /* Return if it's not a type we handle. */
+    if (!IsA(rawreq, SupportRequestInlineSRF))
+        PG_RETURN_POINTER(NULL);
+
+    /* Get things we need off the support request node. */
+    req = (SupportRequestInlineSRF *) rawreq;
+    rtfunc = req->rtfunc;
+    expr = (FuncExpr *) rtfunc->funcexpr;
+
+    /* Generate the SQL string. */
+    initStringInfo(&amp;sql);
+    appendStringInfo(&amp;sql, "SELECT ...");
+
+    /* Build a SQLFunctionParseInfo. */
+    func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(expr->funcid));
+    if (!HeapTupleIsValid(func_tuple))
+    {
+        ereport(WARNING, (errmsg("cache lookup failed for function %u", expr->funcid)));
+        PG_RETURN_POINTER(NULL);
+    }
+    pinfo = prepare_sql_fn_parse_info(func_tuple,
+                                      (Node *) expr,
+                                      expr->inputcollid);
+    ReleaseSysCache(func_tuple);
+
+    /* Parse the SQL. */
+    raw_parsetree_list = pg_parse_query(sql.data);
+    if (list_length(raw_parsetree_list) != 1)
+    {
+        ereport(WARNING, (errmsg("my_support_func parsed to more than one node")));
+        PG_RETURN_POINTER(NULL);
+    }
+
+    /* Analyze the parse tree as if it were a SQL-language body. */
+    querytree_list = pg_analyze_and_rewrite_withcb(linitial(raw_parsetree_list),
+                                                   sql.data,
+                                                   (ParserSetupHook) sql_fn_parser_setup,
+                                                   pinfo, NULL);
+    if (list_length(querytree_list) != 1)
+    {
+        ereport(WARNING, (errmsg("my_support_func rewrote to more than one node")));
+        PG_RETURN_POINTER(NULL);
+    }
+
+    querytree = linitial(querytree_list);
+    if (!IsA(querytree, Query))
+    {
+        ereport(WARNING, (errmsg("my_support_func didn't parse to a Query"),
+                          errdetail("Got this instead: %s", nodeToString(querytree))));
+        PG_RETURN_POINTER(NULL);
+    }
+
+    PG_RETURN_POINTER(querytree);
+}
+</programlisting>
+   </para>
+
    <para>
     For target functions that return <type>boolean</type>, it is often useful to estimate
     the fraction of rows that will be selected by a <literal>WHERE</literal> clause using that
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index a0dc5dfcd61..c9304c2b8fb 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5048,6 +5048,50 @@ evaluate_expr(Expr *expr, Oid result_type, int32 result_typmod,
 }

+/*
+ * inline_set_returning_function_with_support
+ *
+ * This implements inline_set_returning_function for functions with
+ * a support function that can handle SupportRequestInlineSRF.
+ * We let the support function make its own decisions about what it can
+ * handle.  For instance we don't forbid a VOLATILE function or volatile
+ * arguments.
+ */
+static Query *
+inline_set_returning_function_with_support(PlannerInfo *root, RangeTblEntry *rte,
+										   RangeTblFunction *rtfunc,
+										   FuncExpr *fexpr, Form_pg_proc funcform)
+{
+	SupportRequestInlineSRF req;
+	Node	   *newnode;
+
+	/* It must have a support function. */
+	Assert(funcform->prosupport);
+
+	req.root = root;
+	req.type = T_SupportRequestInlineSRF;
+	req.rtfunc = rtfunc;
+	req.proc = funcform;
+
+	/*
+	 * XXX: wrap this in its own memory context, as
+	 * inline_sql_set_returning_function does below?
+	 */
+	newnode = (Node *)
+		DatumGetPointer(OidFunctionCall1(funcform->prosupport,
+										 PointerGetDatum(&req)));
+
+	if (!newnode)
+		return NULL;
+
+	if (!IsA(newnode, Query))
+		elog(ERROR,
+			 "Got unexpected node type %d from %s for function %s",
+			 newnode->type, "SupportRequestInlineSRF", NameStr(funcform->proname));
+
+	return (Query *) newnode;
+}
+
 /*
  * inline_sql_set_returning_function
  *
@@ -5263,10 +5307,10 @@ fail:
 
 /*
  * inline_set_returning_function
- *		Attempt to "inline" an SQL set-returning function in the FROM clause.
+ *		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
+ * set-returning 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
@@ -5285,7 +5329,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	Oid			func_oid;
 	HeapTuple	func_tuple;
 	Form_pg_proc funcform;
-	Query	   *funcquery;
+	Query	   *funcquery = NULL;

@@ -5336,8 +5380,19 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 		elog(ERROR, "cache lookup failed for function %u", func_oid);
 	funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
 
-	funcquery = inline_sql_set_returning_function(root, rte, rtfunc, fexpr,
-												  func_oid, func_tuple, funcform);
+	/*
+	 * If the function has an attached support function that can handle
+	 * SupportRequestInlineSRF, then attempt to inline with that. Return the
+	 * result if we get one, otherwise proceed.
+	 */
+	if (funcform->prosupport)
+		funcquery = inline_set_returning_function_with_support(root, rte, rtfunc, fexpr,
+															   funcform);
+
+	/* Try to inline automatically */
+	if (!funcquery)
+		funcquery = inline_sql_set_returning_function(root, rte, rtfunc, fexpr,
+													  func_oid, func_tuple, funcform);
 
 	if (!funcquery)
 		goto fail;
diff --git a/src/include/nodes/supportnodes.h b/src/include/nodes/supportnodes.h
index 9c047cc401b..2016e8def65 100644
--- a/src/include/nodes/supportnodes.h
+++ b/src/include/nodes/supportnodes.h
@@ -33,6 +33,7 @@
 #ifndef SUPPORTNODES_H
 #define SUPPORTNODES_H
 
+#include "catalog/pg_proc.h"
 #include "nodes/plannodes.h"
 
 struct PlannerInfo;				/* avoid including pathnodes.h here */
@@ -69,6 +70,38 @@ typedef struct SupportRequestSimplify
 	FuncExpr   *fcall;			/* Function call to be simplified */
 } SupportRequestSimplify;
 
+/*
+ * The InlineSRF request allows the support function to perform plan-time
+ * simplification of a call to its target set-returning function. For
+ * example a PL/pgSQL function could build a dynamic SQL query and execute it.
+ * Normally only SQL functions can be inlined, but with this support function
+ * the dynamic query can be inlined as well.
+ *
+ * The planner's PlannerInfo "root" is typically not needed, but can be
+ * consulted if it's necessary to obtain info about Vars present in
+ * the given node tree.  Beware that root could be NULL in some usages.
+ *
+ * "rtfunc" will be a RangeTblFunction node for the function being replaced.
+ * The support function is only called if rtfunc->functions contains a
+ * single FuncExpr node. (ROWS FROM is one way to get more than one.)
+ *
+ * "proc" will be the Form_pg_proc record for the function being replaced.
+ *
+ * The result should be a semantically-equivalent transformed node tree,
+ * or NULL if no simplification could be performed.  It should be allocated
+ * in the CurrentMemoryContext. Do *not* return or modify the FuncExpr node
+ * tree, as it isn't really a separately allocated Node.  But it's okay to
+ * use its args, or parts of it, in the result tree.
+ */
+typedef struct SupportRequestInlineSRF
+{
+	NodeTag		type;
+
+	struct PlannerInfo *root;	/* Planner's infrastructure */
+	RangeTblFunction *rtfunc;	/* Function call to be simplified */
+	Form_pg_proc proc;			/* Function definition */
+}			SupportRequestInlineSRF;
+
 /*
  * The Selectivity request allows the support function to provide a
  * selectivity estimate for a function appearing at top level of a WHERE
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index c3b2b9d8603..13388ebc715 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -601,6 +601,11 @@ CREATE FUNCTION test_support_func(internal)
     RETURNS internal
     AS :'regresslib', 'test_support_func'
     LANGUAGE C STRICT;
+-- With a support function that inlines SRFs
+CREATE FUNCTION test_inline_srf_support_func(internal)
+    RETURNS internal
+    AS :'regresslib', 'test_inline_srf_support_func'
+    LANGUAGE C STRICT;
 ALTER FUNCTION my_int_eq(int, int) SUPPORT test_support_func;
 EXPLAIN (COSTS OFF)
 SELECT * FROM tenk1 a JOIN tenk1 b ON a.unique1 = b.unique1
@@ -777,6 +782,213 @@ false, true, false, true);
  Function Scan on generate_series g  (cost=N..N rows=1000 width=N)
 (1 row)
 
+--
index 3dbba069024..626ec3a1a71 100644
@@ -28,6 +28,7 @@
 #include "postmaster/postmaster.h"	/* for MAX_BACKENDS */
 
 #define EXPECT_TRUE(expr)	\
@@ -803,6 +806,125 @@ test_support_func(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(ret);
 }
 
+PG_FUNCTION_INFO_V1(test_inline_srf_support_func);
+Datum
+test_inline_srf_support_func(PG_FUNCTION_ARGS)
+{
+	Node	   *rawreq = (Node *) PG_GETARG_POINTER(0);
+	Query	   *querytree = NULL;
+
+	if (IsA(rawreq, SupportRequestInlineSRF))
+	{
+		/*
+		 * Assume that the target is foo_from_bar; that's safe as long as we
+		 * don't attach this to any other set-returning function.
+		 */
+		SupportRequestInlineSRF *req = (SupportRequestInlineSRF *) rawreq;
+		StringInfoData sql;
+		RangeTblFunction *rtfunc = req->rtfunc;
+		FuncExpr   *expr = (FuncExpr *) rtfunc->funcexpr;
+		Node	   *node;
+		Const	   *c;
+		char	   *colname;
+		char	   *tablename;
+		HeapTuple	func_tuple;
+		SQLFunctionParseInfoPtr pinfo;
+		List	   *raw_parsetree_list;
+		List	   *querytree_list;
+
+		if (list_length(expr->args) != 3)
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func called with %d args but expected 3", list_length(expr->args))));
+			PG_RETURN_POINTER(NULL);
+		}
+
+		/* Get colname */
+		node = linitial(expr->args);
+		if (!IsA(node, Const))
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func called with non-Const parameters")));
+			PG_RETURN_POINTER(NULL);
+		}
+
+		c = (Const *) node;
+		if (c->consttype != TEXTOID)
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func called with non-TEXT parameters")));
+			PG_RETURN_POINTER(NULL);
+		}
+		colname = TextDatumGetCString(c->constvalue);
+
+		/* Get tablename */
+		node = lsecond(expr->args);
+		if (!IsA(node, Const))
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func called with non-Const parameters")));
+			PG_RETURN_POINTER(NULL);
+		}
+
+		c = (Const *) node;
+		if (c->consttype != TEXTOID)
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func called with non-TEXT parameters")));
+			PG_RETURN_POINTER(NULL);
+		}
+		tablename = TextDatumGetCString(c->constvalue);
+
+		initStringInfo(&sql);
+		appendStringInfo(&sql, "SELECT %s::text FROM %s", quote_identifier(colname), quote_identifier(tablename));
+
+		/* Get filter if present */
+		node = lthird(expr->args);
+		if (!(IsA(node, Const) && ((Const *) node)->constisnull))
+		{
+			appendStringInfo(&sql, " WHERE %s::text = $3", quote_identifier(colname));
+		}
+
+		/* Build a SQLFunctionParseInfo. */
+		func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(expr->funcid));
+		if (!HeapTupleIsValid(func_tuple))
+		{
+			ereport(WARNING, (errmsg("cache lookup failed for function %u", expr->funcid)));
+			PG_RETURN_POINTER(NULL);
+		}
+
+		pinfo = prepare_sql_fn_parse_info(func_tuple,
+										  (Node *) expr,
+										  expr->inputcollid);
+
+		ReleaseSysCache(func_tuple);
+
+		/* Parse the SQL. */
+		raw_parsetree_list = pg_parse_query(sql.data);
+		if (list_length(raw_parsetree_list) != 1)
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func parsed to more than one node")));
+			PG_RETURN_POINTER(NULL);
+		}
+
+		/* Analyze the parse tree as if it were a SQL-language body. */
+		querytree_list = pg_analyze_and_rewrite_withcb(linitial(raw_parsetree_list),
+													   sql.data,
+													   (ParserSetupHook) sql_fn_parser_setup,
+													   pinfo, NULL);
+		if (list_length(querytree_list) != 1)
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func rewrote to more than one node")));
+			PG_RETURN_POINTER(NULL);
+		}
+
+		querytree = linitial(querytree_list);
+		if (!IsA(querytree, Query))
+		{
+			ereport(WARNING, (errmsg("test_inline_srf_support_func didn't parse to a Query"),
+							  errdetail("Got this instead: %s", nodeToString(querytree))));
+			PG_RETURN_POINTER(NULL);
+		}
+	}
+
+	PG_RETURN_POINTER(querytree);
+}
+
 PG_FUNCTION_INFO_V1(test_opclass_options_func);
 Datum
 test_opclass_options_func(PG_FUNCTION_ARGS)
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 23792c4132a..56c6e7a8a88 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -248,6 +248,12 @@ CREATE FUNCTION test_support_func(internal)
     AS :'regresslib', 'test_support_func'
     LANGUAGE C STRICT;

 EXPLAIN (COSTS OFF)
@@ -349,6 +355,86 @@ SELECT explain_mask_costs($$
 SELECT * FROM generate_series(25.0, 2.0, 0.0) g(s);$$,
 false, true, false, true);
 
+--
2.39.5

---

From: Tom Lane <[email-masked]>
Date: Mon, 14 Jul 2025 17:21:23 -0400

> inline_set_returning_function_with_support, then the second patch adds the new functionality.

I got around to looking at this again.  I generally agree with your
approach to the refactoring in clauses.c, with minor nitpicks:

* I don't like postponing the early exit for its-not-a-SELECT;
as coded, this wastes a pretty decent number of cycles transforming
a querytree that won't be used (not to mention that I'm not sure
that our usage of check_sql_fn_retval won't fail on a non-SELECT).
So I think we should keep this bit where it is:

-	/*

and then in the other path, simply Assert that those two conditions
hold for anything the support function might try to give back.

* I'm inclined to think that the test for "it must be declared to
return a set" should stay in inline_sql_set_returning_function.
In the case of a support-function-supported function, it's okay either
if the function returns a set or if it is guaranteed to return exactly
one row (including edge cases such as null function arguments).
The support function either knows that already or can take the
responsibility for checking it.  But if we do it like this, we
foreclose the possibility of supporting the latter class of functions.

* But on the other hand, I wonder if this bit shouldn't move to
the outer function:

    /*
     * 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;

I am not really convinced that any support function could safely
ignore those restrictions, and I do fear that a lot would omit the
enforcement and thereby produce wrong queries in such cases.  Another
thing that likely needs to be in the outer wrapper is the check that
pg_proc_proconfig is empty, because that doesn't seem like a case
that support functions could skip over either.

* I don't like the memory management.  I think creation/destruction
of the temp context should occur at the outer level, and in particular
that we want to perform substitute_actual_srf_parameters() while still
working in the temp context, and copy out only the final form of the
query tree.  This addresses your XXX comment in v2-0002, and also
saves support functions from having to re-invent that wheel.

> I didn't love passing a SysCache HeapTuple into another function,

No, that's perfectly common; see for example
prepare_sql_fn_parse_info.  In fact, one thing I don't like in v2-0002
is that you should pass the pg_proc entry to the support function as a
HeapTuple not Form_pg_proc.  It's possible to get the Form_pg_proc
pointer from the HeapTuple but not vice versa, while the Form_pg_proc
does not allow access to varlena fields, which makes it useless for
many cases.  Even your own example function is forced to re-fetch
the syscache entry because of this.

One other comment on v2-0002 is that this bit doesn't look right:

It's not actually doing anything with the "node" value.

Backing up to a higher level, it seems like we still have no answer
for how to build a valid support function result besides "construct an
equivalent SQL query string and feed it through parse analysis and
rewrite".  That seems both restrictive and expensive.  In particular
it begs the question of why the target function couldn't just have
been written as a SQL function to begin with.  So I still have kind
of a low estimate of this feature's usefulness.  Is there a way to
do better?

			regards, tom lane

---

From: Paul A Jungwirth <[email-masked]>
Date: Fri, 8 Aug 2025 14:46:01 -0700

> approach to the refactoring in clauses.c, with minor nitpicks:

Thanks for taking another look! Revisions attached.

> hold for anything the support function might try to give back.

Okay.

> foreclose the possibility of supporting the latter class of functions.

Makes sense. Done.

> that support functions could skip over either.

Agreed, done.

> saves support functions from having to re-invent that wheel.

Okay, done. I was trying to defer creating a memory context past as
many checks as possible. It's not an expensive thing to do?

> the syscache entry because of this.

Okay, thanks for explaining! Done.

> It's not actually doing anything with the "node" value.

This is correct, but I added a comment. The idea is that $3 will get
the value of "node".

> of a low estimate of this feature's usefulness.  Is there a way to

Parsing the function body is no more expensive than what we'd do to
execute it separately, right? And by inlining we only do it once. But
if it was too much for someone, perhaps they could keep a cache of
node trees based on the function arguments and/or hash of the SQL
string, not unlike how foreign keys cache query plans. Or they could
build the node tree directly, without parsing. But parsing the
equivalent string is easy and covers most uses. I can even imagine a
way to eventually semi-automate it, e.g. creating a general-purpose
support function that you can attach to (many) PL/pgSQL functions that
end in `EXECUTE format(...)`.

The reason for supporting more than SQL functions is to let you
construct the query dynamically, e.g. with user-supplied table/column
names, or to only include some expensive filters if needed. This would
be great for building functions that implement temporal
outer/semi/antijoin. Another use-case I personally have, which I think
is quite common, is building "parameterized views" for permissions
checks, e.g. visible_sales(user). In that case we may only need to
include certain joins if the user belongs to certain roles (e.g. a
third-party sales rep).

Rebased to 04b7ff3cd3.

Yours,

---

From: Tom Lane <[email-masked]>
Date: Sat, 22 Nov 2025 19:44:41 -0500

> include certain joins if the user belongs to certain roles (e.g. a

I went through this again, and committed it with a bunch of
mostly-cosmetic changes.  In particular, it seemed like talking
about inlining "set-returning functions" is no longer really on-point,
since this mechanism is perfectly capable of inlining non-SRFs.
(The reason we haven't done that for SQL functions is mainly that
we didn't feel like doing the analysis necessary to prove that a
SELECT will return exactly one row, which would be necessary to
maintain semantic equivalence for a non-SRF after inlining.
The easy cases of that, such as "SELECT expression", are already
sufficiently handled by regular inlining.)

So after some thought I renamed inline_set_returning_function to
inline_function_in_from, and made a bunch of other changes in names
and comments to line up with that.

Thanks for working on this!  I know it's been a long slog.

			regards, tom lane

---

From: Pavel Stehule <[email-masked]>
Date: Tue, 2 Dec 2025 06:52:21 +0100

Hi

> Thanks for working on this!  I know it's been a long slog.

I checked this patch, and I think so using the body of the
function foo_from_bar is very confusing.
If I understand this patch and functionality, then the function is never
executed (when the support function has support
for SupportRequestInlineInFrom).

+CREATE FUNCTION test_inline_in_from_support_func(internal)
+    RETURNS internal
+    AS :'regresslib', 'test_inline_in_from_support_func'
+    LANGUAGE C STRICT;
+CREATE FUNCTION foo_from_bar(colname TEXT, tablename TEXT, filter TEXT)
+RETURNS SETOF TEXT
+LANGUAGE plpgsql
+AS $function$
+DECLARE
+  sql TEXT;
+BEGIN
+  sql := format('SELECT %I::text FROM %I', colname, tablename);
+  IF filter IS NOT NULL THEN
+    sql := CONCAT(sql, format(' WHERE %I::text = $1', colname));
+  END IF;
+  RETURN QUERY EXECUTE sql USING filter;
+END;
+$function$ STABLE;

More correct form should be

BEGIN
  RAISE EXCEPTION 'halt - should not be executed directly';
END;

Regards

Pavel

---

From: Tom Lane <[email-masked]>
Date: Tue, 02 Dec 2025 01:05:52 -0500

> function foo_from_bar is very confusing.

Perhaps that example could use more documentation effort ...

>   RAISE EXCEPTION 'halt - should not be executed directly';

... but I don't agree with this at all.  We specifically do
not guarantee that replacement via SupportRequestSimplify
will be honored, and I think the same must be true for
SupportRequestInlineInFrom.  Otherwise we risk breaking
who-knows-how-many third-party tools, as well as cases
where the optimizer declines to inline (eg, because there
are volatile arguments).  Besides, what's the value?

			regards, tom lane

---

From: Pavel Stehule <[email-masked]>
Date: Tue, 2 Dec 2025 07:14:06 +0100

> are volatile arguments).  Besides, what's the value?

My note is just for this test.

I understand that the planner function can handle only some specific
variants - and all others will be done by real execution.

But this is really confusing - I read the blog
https://www.thatguyfromdelhi.com/2025/11/teaching-query-planner-to-see-inside-c.html
and I was very interested in what is magic in forward, and the reality is
very simple.

I think this feature can be very practical - but can be very messy for
people who don't understand well to complete the picture. And it is a
second level of overloading (what is byself dangerous for someone).

Regards

Pavel

---

From: "zengman" <[email-masked]>
Date: Sat, 20 Dec 2025 12:23:41 +0800

Hi everyone,

I've noticed this interesting feature and would like to modify this test case to clarify that we fall back to the original logic when inlining fails. 
This is a small change that doesn't touch core code. What do you all think?

Regards
Man Zeng

---

From: Pavel Stehule <[email-masked]>
Date: Sat, 20 Dec 2025 07:57:25 +0100

so 20. 12. 2025 v 5:23 odesílatel zengman <[email-masked]> napsal:

> Hi everyone,
>
> I've noticed this interesting feature and would like to modify this test
> case to clarify that we fall back to the original logic when inlining
> fails.

+1

I think so message of warning should be enhanced -

"test_inline_in_from_support_func called with non-TEXT parameters" is messy
little bit.

Regards

Pavel

---

From: "zengman" <[email-masked]>
Date: Sat, 20 Dec 2025 18:53:17 +0800

I share this opinion. In fact, in the existing tests, if test_inline_in_from_support_func is triggered, 
the input parameters are necessarily of type TEXT—so there’s no need for an additional type check.

How about changing it to the following?
```
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index b7a926c6f01..adb8b9793ba 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -849,9 +849,9 @@ test_inline_in_from_support_func(PG_FUNCTION_ARGS)
                }
 
                c = (Const *) node;
-               if (c->consttype != TEXTOID || c->constisnull)
+               if (c->constisnull)
                {
-                       ereport(WARNING, (errmsg("test_inline_in_from_support_func called with non-TEXT parameters")));
+                       ereport(WARNING, (errmsg("test_inline_in_from_support_func called with null parameter for colname")));
                        PG_RETURN_POINTER(NULL);
                }
                colname = TextDatumGetCString(c->constvalue);
@@ -865,9 +865,9 @@ test_inline_in_from_support_func(PG_FUNCTION_ARGS)
                }
 
                c = (Const *) node;
-               if (c->consttype != TEXTOID || c->constisnull)
+               if (c->constisnull)
                {
-                       ereport(WARNING, (errmsg("test_inline_in_from_support_func called with non-TEXT parameters")));
+                       ereport(WARNING, (errmsg("test_inline_in_from_support_func called with null parameter for tablename")));
                        PG_RETURN_POINTER(NULL);
                }
                tablename = TextDatumGetCString(c->constvalue);
```

Regards
Man Zeng

---

From: Pavel Stehule <[email-masked]>
Date: Sat, 20 Dec 2025 12:36:11 +0100

so 20. 12. 2025 v 11:53 odesílatel zengman <[email-masked]> napsal:

> I share this opinion. In fact, in the existing tests, if
> test_inline_in_from_support_func is triggered,
> -                       ereport(WARNING,
> (errmsg("test_inline_in_from_support_func called with non-TEXT
> parameters")));
> +                       ereport(WARNING,
> (errmsg("test_inline_in_from_support_func called with null parameter for
> colname")));
>                         PG_RETURN_POINTER(NULL);
>                 }
> -                       ereport(WARNING,
> (errmsg("test_inline_in_from_support_func called with non-TEXT
> parameters")));
> +                       ereport(WARNING,
> (errmsg("test_inline_in_from_support_func called with null parameter for
> tablename")));
>                         PG_RETURN_POINTER(NULL);
>                 }

+1

Pavel

---

From: "zengman" <[email-masked]>
Date: Sat, 20 Dec 2025 20:21:39 +0800

Thank you for your guidance. Attached is the updated patch.
I wonder if there are any other adjustments needed and look forward to more feedback.

-- 
Regards
Man Zeng
www.openhalo.org

---

From: Tom Lane <[email-masked]>
Date: Sat, 20 Dec 2025 16:37:39 -0500

> This is a small change that doesn't touch core code. What do you all think?

I think this looks like a waste of test cycles.  AFAICS it won't add
even a single line of code coverage for inline_function_in_from().
Why do you think we need it?

			regards, tom lane

---

From: "zengman" <[email-masked]>
Date: Sun, 21 Dec 2025 10:48:07 +0800

Well, from my perspective, the test logic for this feature seems a bit odd: 
even if you directly modify the function body to return NULL; or return 1/0;, 
none of this execution logic takes effect at all—it serves no purpose whatsoever as long as inlining succeeds.
This could be unfriendly to developers who are new to this feature, 
so I added a test for this scenario to verify that our logic falls back to the function body when inlining fails. 
This may just be my personal opinion, though; if you think everything is fine, we can leave it as it is.

--
Regards,
Man Zeng
www.openhalo.org