Remove unnecessary check on set-returning functions in values_lists

[Andrei Lepikhov]

Hi,

I would like to understand the pull_up_simple_values code a little bit more.
Pull-up of simple values was implemented in 2015 by commit f4abd02. In 
the is_simple_values I see a check on the expression_returns_set() of 
the RTE values list.

But since d43a619 in 2017 the check_srf_call_placement has reported an 
ERROR in the case of set-returning function inside a VALUES expression. 
Let's demonstrate:

SELECT * FROM (VALUES ((generate_series(1,1E2))));
ERROR:  set-returning functions are not allowed in VALUES
LINE 1: SELECT * FROM (VALUES ((generate_series(1,1E2))));

I think, the expression_returns_set examination doesn't necessary and we 
can replace it with an assertion, if needed (see attachment).
Am I wrong?

--
regards, Andrei LepikhovFrom 42dc626cf8d41cecb3f88deb578d9cac63934a9c Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <lepi...@gmail.com>
Date: Wed, 28 Aug 2024 16:19:16 +0200
Subject: [PATCH] Remove unnecessary check on set-returning functions in
 values_lists.

Probing the VALUES RTE in the is_simple_values routine it is not necessary
because parser has done that before. Just to be paranoid, check it inside the
assertion.
---
 src/backend/optimizer/prep/prepjointree.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/backend/optimizer/prep/prepjointree.c 
b/src/backend/optimizer/prep/prepjointree.c
index 969e257f70..a465c559e7 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -1783,13 +1783,15 @@ is_simple_values(PlannerInfo *root, RangeTblEntry *rte)
         * about validity of PHVs for the VALUES' outputs.
         */
 
+       /* To be paranoid, let's recheck functions inside the VALUES */
+       Assert(!expression_returns_set((Node *) rte->values_lists));
+
        /*
-        * Don't pull up a VALUES that contains any set-returning or volatile
+        * Don't pull up a VALUES that contains any volatile
         * functions.  The considerations here are basically identical to the
         * restrictions on a pull-able subquery's targetlist.
         */
-       if (expression_returns_set((Node *) rte->values_lists) ||
-               contain_volatile_functions((Node *) rte->values_lists))
+       if (contain_volatile_functions((Node *) rte->values_lists))
                return false;
 
        /*
-- 
2.46.0