Fix no-longer-valid shortcuts in expression_returns_set().

expression_returns_set() used to short-circuit its recursion upon
seeing certain node types, such as DistinctExpr, that it knew the
executor did not support set-valued arguments for.  That was never
inherent, though, just a reflection of laziness in execQual.c.
With the new implementation of SRFs there is no reason to think
that any scalar-valued expression node could not have a set-valued
subexpression, except for AggRefs and WindowFuncs where we know there
is a parser check rejecting it.  And indeed, the shortcut causes
unexpected failures for cases such as a SRF underneath DistinctExpr,
because the planner stops looking for SRFs too soon.

Discussion: https://postgr.es/m/5259.1497044025@sss.pgh.pa.us

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 41f3408cfcf9bf2de4baaf51558fe70bd9fc89e4..5af37a938850f57dadc19de47722761ec57ce3b3 100644 (file)
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -694,39 +694,11 @@ expression_returns_set_walker(Node *node, void *context)
                /* else fall through to check args */
        }
 
-       /* Avoid recursion for some cases that can't return a set */
+       /* Avoid recursion for some cases that parser checks not to return a set */
        if (IsA(node, Aggref))
                return false;
        if (IsA(node, WindowFunc))
                return false;
-       if (IsA(node, DistinctExpr))
-               return false;
-       if (IsA(node, NullIfExpr))
-               return false;
-       if (IsA(node, ScalarArrayOpExpr))
-               return false;
-       if (IsA(node, BoolExpr))
-               return false;
-       if (IsA(node, SubLink))
-               return false;
-       if (IsA(node, SubPlan))
-               return false;
-       if (IsA(node, AlternativeSubPlan))
-               return false;
-       if (IsA(node, ArrayExpr))
-               return false;
-       if (IsA(node, RowExpr))
-               return false;
-       if (IsA(node, RowCompareExpr))
-               return false;
-       if (IsA(node, CoalesceExpr))
-               return false;
-       if (IsA(node, MinMaxExpr))
-               return false;
-       if (IsA(node, SQLValueFunction))
-               return false;
-       if (IsA(node, XmlExpr))
-               return false;
 
        return expression_tree_walker(node, expression_returns_set_walker,
                                                                  context);

diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index ef02ba746555404a449f6ac5563de0c6db9f4999..b691abe714666471435c03770c17229c7741e9bb 100644 (file)
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -443,6 +443,14 @@ SELECT int4mul(generate_series(1,2), 10);
       20
 (2 rows)
 
+SELECT generate_series(1,3) IS DISTINCT FROM 2;
+ ?column? 
+----------
+ t
+ f
+ t
+(3 rows)
+
 -- but SRFs in function RTEs must be at top level (annoying restriction)
 SELECT * FROM int4mul(generate_series(1,2), 10);
 ERROR:  set-returning functions must appear at top level of FROM

diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql
index 45de099e4dc9a91f40100de564481de2521464bc..0be7530beb4de7564b5fc07d86669ba2c4afb294 100644 (file)
--- a/src/test/regress/sql/tsrf.sql
+++ b/src/test/regress/sql/tsrf.sql
@@ -98,6 +98,7 @@ VALUES(1, generate_series(1,2));
 
 -- We allow tSRFs that are not at top level
 SELECT int4mul(generate_series(1,2), 10);
+SELECT generate_series(1,3) IS DISTINCT FROM 2;
 
 -- but SRFs in function RTEs must be at top level (annoying restriction)
 SELECT * FROM int4mul(generate_series(1,2), 10);