Thread: pgsql: Improve parser's and planner's handling of set-returning functio
Improve parser's and planner's handling of set-returning functions. 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> Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/a4c35ea1c2f05dd5b56739fcd0dc36a4870ea0c0 Modified Files -------------- src/backend/catalog/heap.c | 9 +- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/path/allpaths.c | 6 +- src/backend/optimizer/plan/analyzejoins.c | 7 +- src/backend/optimizer/plan/planner.c | 20 ++-- src/backend/optimizer/plan/subselect.c | 10 +- src/backend/optimizer/prep/prepjointree.c | 18 +--- src/backend/optimizer/util/clauses.c | 17 ++-- src/backend/parser/analyze.c | 7 +- src/backend/parser/parse_func.c | 148 ++++++++++++++++++++++++++++++ src/backend/parser/parse_oper.c | 4 + src/backend/parser/parse_utilcmd.c | 20 +--- src/backend/rewrite/rewriteHandler.c | 2 +- src/include/catalog/catversion.h | 2 +- src/include/nodes/parsenodes.h | 1 + src/include/parser/parse_func.h | 2 + src/include/parser/parse_node.h | 3 +- src/pl/plpgsql/src/pl_exec.c | 1 + src/test/regress/expected/tsrf.out | 13 ++- src/test/regress/sql/tsrf.sql | 4 +- 23 files changed, 225 insertions(+), 73 deletions(-)
Re: pgsql: Improve parser's and planner's handling of set-returning functio
From
Peter Geoghegan
Date:
On Tue, Sep 13, 2016 at 10:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > 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. There is a reference to this right at the end of the executor README that was missed. -- Peter Geoghegan
Peter Geoghegan <pg@heroku.com> writes: > There is a reference to this right at the end of the executor README > that was missed. Yeah, Andres already reminded me ;-). Fixed. regards, tom lane