Improve parser's and planner's handling of set-returning functions.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Sep 2016 17:54:24 +0000 (13:54 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 13 Sep 2016 17:54:24 +0000 (13:54 -0400)
Teach the parser to reject misplaced set-returning functions during parse
analysis using p_expr_kind, in much the same way as we do for aggregates
and window functions (cf commit eaccfded9).  While this isn't complete
(it misses nesting-based restrictions), it's much better than the previous
error reporting for such cases, and it allows elimination of assorted
ad-hoc expression_returns_set() error checks.  We could add nesting checks
later if it seems important to catch all cases at parse time.

There is one case the parser will now throw error for although previous
versions allowed it, which is SRFs in the tlist of an UPDATE.  That never
behaved sensibly (since it's ill-defined which generated row should be
used to perform the update) and it's hard to see why it should not be
treated as an error.  It's a release-note-worthy change though.

Also, add a new Query field hasTargetSRFs reporting whether there are
any SRFs in the targetlist (including GROUP BY/ORDER BY expressions).
The parser can now set that basically for free during parse analysis,
and we can use it in a number of places to avoid expression_returns_set
searches.  (There will be more such checks soon.)  In some places, this
allows decontorting the logic since it's no longer expensive to check for
SRFs in the tlist --- so I made the checks parallel to the handling of
hasAggs/hasWindowFuncs wherever it seemed appropriate.

catversion bump because adding a Query field changes stored rules.

Andres Freund and Tom Lane

Discussion: <24639.1473782855@sss.pgh.pa.us>

23 files changed:
src/backend/catalog/heap.c
src/backend/nodes/copyfuncs.c
src/backend/nodes/equalfuncs.c
src/backend/nodes/outfuncs.c
src/backend/nodes/readfuncs.c
src/backend/optimizer/path/allpaths.c
src/backend/optimizer/plan/analyzejoins.c
src/backend/optimizer/plan/planner.c
src/backend/optimizer/plan/subselect.c
src/backend/optimizer/prep/prepjointree.c
src/backend/optimizer/util/clauses.c
src/backend/parser/analyze.c
src/backend/parser/parse_func.c
src/backend/parser/parse_oper.c
src/backend/parser/parse_utilcmd.c
src/backend/rewrite/rewriteHandler.c
src/include/catalog/catversion.h
src/include/nodes/parsenodes.h
src/include/parser/parse_func.h
src/include/parser/parse_node.h
src/pl/plpgsql/src/pl_exec.c
src/test/regress/expected/tsrf.out
src/test/regress/sql/tsrf.sql

index e997b574ca9eaf93514817464b3a77467d1a5371..dbd609493f91ee3f7e27fe62daae1fe5c909e3b4 100644 (file)
@@ -2560,14 +2560,9 @@ cookDefault(ParseState *pstate,
 
        /*
         * transformExpr() should have already rejected subqueries, aggregates,
-        * and window functions, based on the EXPR_KIND_ for a default expression.
-        *
-        * It can't return a set either.
+        * window functions, and SRFs, based on the EXPR_KIND_ for a default
+        * expression.
         */
-       if (expression_returns_set(expr))
-               ereport(ERROR,
-                               (errcode(ERRCODE_DATATYPE_MISMATCH),
-                                errmsg("default expression must not return a set")));
 
        /*
         * Coerce the expression to the correct type and typmod, if given. This
index 4f39dad66b4ec5e483cd8d08692f5aa42834dd1a..71714bc1d6709617787f06842c4690d6d0edf54d 100644 (file)
@@ -2731,6 +2731,7 @@ _copyQuery(const Query *from)
        COPY_SCALAR_FIELD(resultRelation);
        COPY_SCALAR_FIELD(hasAggs);
        COPY_SCALAR_FIELD(hasWindowFuncs);
+       COPY_SCALAR_FIELD(hasTargetSRFs);
        COPY_SCALAR_FIELD(hasSubLinks);
        COPY_SCALAR_FIELD(hasDistinctOn);
        COPY_SCALAR_FIELD(hasRecursive);
index 4800165a919f3cbe447dfb199030354d76497ad8..29a090fc48b60349d87c55fd6354fa4031463fbd 100644 (file)
@@ -921,6 +921,7 @@ _equalQuery(const Query *a, const Query *b)
        COMPARE_SCALAR_FIELD(resultRelation);
        COMPARE_SCALAR_FIELD(hasAggs);
        COMPARE_SCALAR_FIELD(hasWindowFuncs);
+       COMPARE_SCALAR_FIELD(hasTargetSRFs);
        COMPARE_SCALAR_FIELD(hasSubLinks);
        COMPARE_SCALAR_FIELD(hasDistinctOn);
        COMPARE_SCALAR_FIELD(hasRecursive);
index 90fecb1338d361a6e7acb3b1c7cf44283fbdb35a..7e092d700c5fea3f40676b2b3dcec6d16dfac956 100644 (file)
@@ -2683,6 +2683,7 @@ _outQuery(StringInfo str, const Query *node)
        WRITE_INT_FIELD(resultRelation);
        WRITE_BOOL_FIELD(hasAggs);
        WRITE_BOOL_FIELD(hasWindowFuncs);
+       WRITE_BOOL_FIELD(hasTargetSRFs);
        WRITE_BOOL_FIELD(hasSubLinks);
        WRITE_BOOL_FIELD(hasDistinctOn);
        WRITE_BOOL_FIELD(hasRecursive);
index 894a48fb4fcd98086f772b56d60f631b53c3b92c..917e6c8a65efe96aa84e51780f702376bb70473c 100644 (file)
@@ -238,6 +238,7 @@ _readQuery(void)
        READ_INT_FIELD(resultRelation);
        READ_BOOL_FIELD(hasAggs);
        READ_BOOL_FIELD(hasWindowFuncs);
+       READ_BOOL_FIELD(hasTargetSRFs);
        READ_BOOL_FIELD(hasSubLinks);
        READ_BOOL_FIELD(hasDistinctOn);
        READ_BOOL_FIELD(hasRecursive);
index 04264b43359f37330d7ba42d7ccb2e5d48a16838..99b6bc8f5a19781cc62202736bfb5ec309f013f1 100644 (file)
@@ -2422,7 +2422,8 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
                        continue;
 
                /* Functions returning sets are unsafe (point 1