diff options
Diffstat (limited to 'src')
23 files changed, 225 insertions, 73 deletions
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index e997b574ca9..dbd609493f9 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -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 diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 4f39dad66b4..71714bc1d67 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -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); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 4800165a919..29a090fc48b 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -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); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 90fecb1338d..7e092d700c5 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -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); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 894a48fb4fc..917e6c8a65e 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -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); diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 04264b43359..99b6bc8f5a1 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -2422,7 +2422,8 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo) continue; /* Functions returning sets are unsafe (point 1) */ - if (expression_returns_set((Node *) tle->expr)) + if (subquery->hasTargetSRFs && + expression_returns_set((Node *) tle->expr)) { safetyInfo->unsafeColumns[tle->resno] = true; continue; @@ -2835,7 +2836,8 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel) * If it contains a set-returning function, we can't remove it since * that could change the number of rows returned by the subquery. */ - if (expression_returns_set(texpr)) + if (subquery->hasTargetSRFs && + expression_returns_set(texpr)) continue; /* diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index e28a8dc533e..74e42451228 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -650,6 +650,11 @@ rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, List *clause_list) bool query_supports_distinctness(Query *query) { + /* we don't cope with SRFs, see comment below */ + if (query->hasTargetSRFs) + return false; + + /* check for features we can prove distinctness with */ if (query->distinctClause != NIL || query->groupClause != NIL || query->groupingSets != NIL || @@ -695,7 +700,7 @@ query_is_distinct_for(Query *query, List *colnos, List *opids) * specified columns, since those must be evaluated before de-duplication; * but it doesn't presently seem worth the complication to check that.) */ - if (expression_returns_set((Node *) query->targetList)) + if (query->hasTargetSRFs) return false; /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 174210be6cf..f657ffc4463 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -604,6 +604,10 @@ subquery_planner(PlannerGlobal *glob, Query *parse, preprocess_expression(root, (Node *) parse->targetList, EXPRKIND_TARGET); + /* Constant-folding might have removed all set-returning functions */ + if (parse->hasTargetSRFs) + parse->hasTargetSRFs = expression_returns_set((Node *) parse->targetList); + newWithCheckOptions = NIL; foreach(l, parse->withCheckOptions) { @@ -1702,16 +1706,14 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, * Figure out whether there's a hard limit on the number of rows that * query_planner's result subplan needs to return. Even if we know a * hard limit overall, it doesn't apply if the query has any - * grouping/aggregation operations. (XXX it also doesn't apply if the - * tlist contains any SRFs; but checking for that here seems more - * costly than it's worth, since root->limit_tuples is only used for - * cost estimates, and only in a small number of cases.) + * grouping/aggregation operations, or SRFs in the tlist. */ if (parse->groupClause || parse->groupingSets || parse->distinctClause || parse->hasAggs || parse->hasWindowFuncs || + parse->hasTargetSRFs || root->hasHavingQual) root->limit_tuples = -1.0; else @@ -1928,7 +1930,11 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, * weird usage that it doesn't seem worth greatly complicating matters to * account for it. */ - tlist_rows = tlist_returns_set_rows(tlist); + if (parse->hasTargetSRFs) + tlist_rows = tlist_returns_set_rows(tlist); + else + tlist_rows = 1; + if (tlist_rows > 1) { foreach(lc, current_rel->pathlist) @@ -4995,7 +5001,8 @@ make_sort_input_target(PlannerInfo *root, * Check for SRF or volatile functions. Check the SRF case first * because we must know whether we have any postponed SRFs. */ - if (expression_returns_set((Node *) expr)) + if (parse->hasTargetSRFs && + expression_returns_set((Node *) expr)) { /* We'll decide below whether these are postponable */ col_is_srf[i] = true; @@ -5034,6 +5041,7 @@ make_sort_input_target(PlannerInfo *root, { /* For sortgroupref cols, just check if any contain SRFs */ if (!have_srf_sortcols && + parse->hasTargetSRFs && expression_returns_set((Node *) expr)) have_srf_sortcols = true; } diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 6edefb11388..263ba45f9f4 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1562,7 +1562,7 @@ simplify_EXISTS_query(PlannerInfo *root, Query *query) { /* * We don't try to simplify at all if the query uses set operations, - * aggregates, grouping sets, modifying CTEs, HAVING, OFFSET, or FOR + * aggregates, grouping sets, SRFs, modifying CTEs, HAVING, OFFSET, or FOR * UPDATE/SHARE; none of these seem likely in normal usage and their * possible effects are complex. (Note: we could ignore an "OFFSET 0" * clause, but that traditionally is used as an optimization fence, so we @@ -1573,6 +1573,7 @@ simplify_EXISTS_query(PlannerInfo *root, Query *query) query->hasAggs || query->groupingSets || query->hasWindowFuncs || + query->hasTargetSRFs || query->hasModifyingCTE || query->havingQual || query->limitOffset || @@ -1614,13 +1615,6 @@ simplify_EXISTS_query(PlannerInfo *root, Query *query) } /* - * Mustn't throw away the targetlist if it contains set-returning - * functions; those could affect whether zero rows are returned! - */ - if (expression_returns_set((Node *) query->targetList)) - return false; - - /* * Otherwise, we can throw away the targetlist, as well as any GROUP, * WINDOW, DISTINCT, and ORDER BY clauses; none of those clauses will * change a nonzero-rows result to zero rows or vice versa. (Furthermore, diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index a334f15773a..878db9b4ab7 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -1188,8 +1188,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, parse->hasSubLinks |= subquery->hasSubLinks; /* - * subquery won't be pulled up if it hasAggs or hasWindowFuncs, so no work - * needed on those flags + * subquery won't be pulled up if it hasAggs, hasWindowFuncs, or + * hasTargetSRFs, so no work needed on those flags */ /* @@ -1419,8 +1419,8 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, return false; /* - * Can't pull up a subquery involving grouping, aggregation, sorting, - * limiting, or WITH. (XXX WITH could possibly be allowed later) + * Can't pull up a subquery involving grouping, aggregation, SRFs, + * sorting, limiting, or WITH. (XXX WITH could possibly be allowed later) * * We also don't pull up a subquery that has explicit FOR UPDATE/SHARE * clauses, because pullup would cause the locking to occur semantically @@ -1430,6 +1430,7 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, */ if (subquery->hasAggs || subquery->hasWindowFuncs || + subquery->hasTargetSRFs || subquery->groupClause || subquery->groupingSets || subquery->havingQual || @@ -1543,15 +1544,6 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, } /* - * Don't pull up a subquery that has any set-returning functions in its - * targetlist. Otherwise we might well wind up inserting set-returning - * functions into places where they mustn't go, such as quals of higher - * queries. This also ensures deletion of an empty jointree is valid. - */ - if (expression_returns_set((Node *) subquery->targetList)) - return false; - - /* * Don't pull up a subquery that has any volatile functions in its * targetlist. Otherwise we might introduce multiple evaluations of these * functions, if they get copied to multiple places in the upper query, diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index e1baf71e383..663ffe05352 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -4449,6 +4449,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, querytree->utilityStmt || querytree->hasAggs || querytree->hasWindowFuncs || + querytree->hasTargetSRFs || querytree->hasSubLinks || querytree->cteList || querytree->rtable || @@ -4489,17 +4490,13 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, Assert(!modifyTargetList); /* - * Additional validity checks on the expression. It mustn't return a set, - * and it mustn't be more volatile than the surrounding function (this is - * to avoid breaking hacks that involve pretending a function is immutable - * when it really ain't). If the surrounding function is declared strict, - * then the expression must contain only strict constructs and must use - * all of the function parameters (this is overkill, but an exact analysis - * is hard). + * Additional validity checks on the expression. It mustn't be more + * volatile than the surrounding function (this is to avoid breaking hacks + * that involve pretending a function is immutable when it really ain't). + * If the surrounding function is declared strict, then the expression + * must contain only strict constructs and must use all of the function + * parameters (this is overkill, but an exact analysis is hard). */ - if (expression_returns_set(newexpr)) - goto fail; - if (funcform->provolatile == PROVOLATILE_IMMUTABLE && contain_mutable_functions(newexpr)) goto fail; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index eac86cce3ee..870fae3f51b 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -417,6 +417,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasWindowFuncs = pstate->p_hasWindowFuncs; + qry->hasTargetSRFs = pstate->p_hasTargetSRFs; qry->hasAggs = pstate->p_hasAggs; if (pstate->p_hasAggs) parseCheckAggregates(pstate, qry); @@ -819,6 +820,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) qry->rtable = pstate->p_rtable; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); + qry->hasTargetSRFs = pstate->p_hasTargetSRFs; qry->hasSubLinks = pstate->p_hasSubLinks; assign_query_collations(pstate, qry); @@ -1231,6 +1233,7 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasWindowFuncs = pstate->p_hasWindowFuncs; + qry->hasTargetSRFs = pstate->p_hasTargetSRFs; qry->hasAggs = pstate->p_hasAggs; if (pstate->p_hasAggs || qry->groupClause || qry->groupingSets || qry->havingQual) parseCheckAggregates(pstate, qry); @@ -1691,6 +1694,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasWindowFuncs = pstate->p_hasWindowFuncs; + qry->hasTargetSRFs = pstate->p_hasTargetSRFs; qry->hasAggs = pstate->p_hasAggs; if (pstate->p_hasAggs || qry->groupClause || qry->groupingSets || qry->havingQual) parseCheckAggregates(pstate, qry); @@ -2170,6 +2174,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) qry->rtable = pstate->p_rtable; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); + qry->hasTargetSRFs = pstate->p_hasTargetSRFs; qry->hasSubLinks = pstate->p_hasSubLinks; assign_query_collations(pstate, qry); @@ -2565,7 +2570,7 @@ CheckSelectLocking(Query *qry, LockClauseStrength strength) translator: %s is a SQL row locking clause such as FOR UPDATE */ errmsg("%s is not allowed with window functions", LCS_asString(strength)))); - if (expression_returns_set((Node *) qry->targetList)) + if (qry->hasTargetSRFs) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /*------ diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 61af484feeb..56c9a4293df 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -25,6 +25,7 @@ #include "parser/parse_agg.h" #include "parser/parse_clause.h" #include "parser/parse_coerce.h" +#include "parser/parse_expr.h" #include "parser/parse_func.h" #include "parser/parse_relation.h" #include "parser/parse_target.h" @@ -625,6 +626,10 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, exprLocation((Node *) llast(fargs))))); } + /* if it returns a set, check that's OK */ + if (retset) + check_srf_call_placement(pstate, location); + /* build the appropriate output structure */ if (fdresult == FUNCDETAIL_NORMAL) { @@ -2040,3 +2045,146 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) return oid; } + + +/* + * check_srf_call_placement + * Verify that a set-returning function is called in a valid place, + * and throw a nice error if not. + * + * A side-effect is to set pstate->p_hasTargetSRFs true if appropriate. + */ +void +check_srf_call_placement(ParseState *pstate, int location) +{ + const char *err; + bool errkind; + + /* + * Check to see if the set-returning function is in an invalid place + * within the query. Basically, we don't allow SRFs anywhere except in + * the targetlist (which includes GROUP BY/ORDER BY expressions), VALUES, + * and functions in FROM. + * + * For brevity we support two schemes for reporting an error here: set + * "err" to a custom message, or set "errkind" true if the error context + * is sufficiently identified by what ParseExprKindName will return, *and* + * what it will return is just a SQL keyword. (Otherwise, use a custom + * message to avoid creating translation problems.) + */ + err = NULL; + errkind = false; + switch (pstate->p_expr_kind) + { + case EXPR_KIND_NONE: + Assert(false); /* can't happen */ + break; + case EXPR_KIND_OTHER: + /* Accept SRF here; caller must throw error if wanted */ + break; + case EXPR_KIND_JOIN_ON: + case EXPR_KIND_JOIN_USING: + err = _("set-returning functions are not allowed in JOIN conditions"); + break; + case EXPR_KIND_FROM_SUBSELECT: + /* can't get here, but just in case, throw an error */ + errkind = true; + break; + case EXPR_KIND_FROM_FUNCTION: + /* okay ... but we can't check nesting here */ + break; + case EXPR_KIND_WHERE: + errkind = true; + break; + case EXPR_KIND_POLICY: + err = _("set-returning functions are not allowed in policy expressions"); + break; + case EXPR_KIND_HAVING: + errkind = true; + break; + case EXPR_KIND_FILTER: + errkind = true; + break; + case EXPR_KIND_WINDOW_PARTITION: + case EXPR_KIND_WINDOW_ORDER: + /* okay, these are effectively GROUP BY/ORDER BY */ + pstate->p_hasTargetSRFs = true; + break; + case EXPR_KIND_WINDOW_FRAME_RANGE: + case EXPR_KIND_WINDOW_FRAME_ROWS: + err = _("set-returning functions are not allowed in window definitions"); + break; + case EXPR_KIND_SELECT_TARGET: + case EXPR_KIND_INSERT_TARGET: + /* okay */ + pstate->p_hasTargetSRFs = true; + break; + case EXPR_KIND_UPDATE_SOURCE: + case EXPR_KIND_UPDATE_TARGET: + /* disallowed because it would be ambiguous what to do */ + errkind = true; + break; + case EXPR_KIND_GROUP_BY: + case EXPR_KIND_ORDER_BY: + /* okay */ + pstate->p_hasTargetSRFs = true; + break; + case EXPR_KIND_DISTINCT_ON: + /* okay */ + pstate->p_hasTargetSRFs = true; + break; + case EXPR_KIND_LIMIT: + case EXPR_KIND_OFFSET: + errkind = true; + break; + case EXPR_KIND_RETURNING: + errkind = true; + break; + case EXPR_KIND_VALUES: + /* okay */ + break; + case EXPR_KIND_CHECK_CONSTRAINT: + case EXPR_KIND_DOMAIN_CHECK: + err = _("set-returning functions are not allowed in check constraints"); + break; + case EXPR_KIND_COLUMN_DEFAULT: + case EXPR_KIND_FUNCTION_DEFAULT: + err = _("set-returning functions are not allowed in DEFAULT expressions"); + break; + case EXPR_KIND_INDEX_EXPRESSION: + err = _("set-returning functions are not allowed in index expressions"); + break; + case EXPR_KIND_INDEX_PREDICATE: + err = _("set-returning functions are not allowed in index predicates"); + break; + case EXPR_KIND_ALTER_COL_TRANSFORM: + err = _("set-returning functions are not allowed in transform expressions"); + break; + case EXPR_KIND_EXECUTE_PARAMETER: + err = _("set-returning functions are not allowed in EXECUTE parameters"); + break; + case EXPR_KIND_TRIGGER_WHEN: + err = _("set-returning functions are not allowed in trigger WHEN conditions"); + break; + + /* + * There is intentionally no default: case here, so that the + * compiler will warn if we add a new ParseExprKind without + * extending this switch. If we do see an unrecognized value at + * runtime, the behavior will be the same as for EXPR_KIND_OTHER, + * which is sane anyway. + */ + } + if (err) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg_internal("%s", err), + parser_errposition(pstate, location))); + if (errkind) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s is name of a SQL construct, eg GROUP BY */ + errmsg("set-returning functions are not allowed in %s", + ParseExprKindName(pstate->p_expr_kind)), + parser_errposition(pstate, location))); +} diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c index e913d05a793..aecda6d9339 100644 --- a/src/backend/parser/parse_oper.c +++ b/src/backend/parser/parse_oper.c @@ -839,6 +839,10 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree, result->args = args; result->location = location; + /* if it returns a set, check that's OK */ + if (result->opretset) + check_srf_call_placement(pstate, location); + ReleaseSysCache(tup); return (Expr *) result; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 7a2950e6a92..0670bc24822 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -294,7 +294,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) * overridden if an inherited table has oids. */ stmt->options = lcons(makeDefElem("oids", - (Node *) makeInteger(cxt.hasoids), -1), + (Node *) makeInteger(cxt.hasoids), -1), stmt->options); } @@ -483,7 +483,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) makeString(cxt->relation->relname), makeString(column->colname)); altseqstmt->options = list_make1(makeDefElem("owned_by", - (Node *) attnamelist, -1)); + (Node *) attnamelist, -1)); cxt->alist = lappend(cxt->alist, altseqstmt); @@ -2106,17 +2106,11 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString) /* * transformExpr() should have already rejected subqueries, - * aggregates, and window functions, based on the EXPR_KIND_ for - * an index expression. + * aggregates, window functions, and SRFs, based on the EXPR_KIND_ + * for an index expression. * - * Also reject expressions returning sets; this is for consistency - * with what transformWhereClause() checks for the predicate. * DefineIndex() will make more checks. */ - if (expression_returns_set(ielem->expr)) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("index expression cannot return a set"))); } } @@ -2594,12 +2588,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, def->cooked_default = transformExpr(pstate, def->raw_default, EXPR_KIND_ALTER_COL_TRANSFORM); - - /* it can't return a set */ - if (expression_returns_set(def->cooked_default)) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("transform expression must not return a set"))); } newcmds = lappend(newcmds, cmd); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index a22a11e2c19..b828e3cb077 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -2221,7 +2221,7 @@ view_query_is_auto_updatable(Query *viewquery, bool check_cols) if (viewquery->hasWindowFuncs) return gettext_noop("Views that return window functions are not automatically updatable."); - if (expression_returns_set((Node *) viewquery->targetList)) + if (viewquery->hasTargetSRFs) return gettext_noop("Views that return set-returning functions are not automatically updatable."); /* diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index c04edadbf09..ef691c5721f 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201608231 +#define CATALOG_VERSION_NO 201609131 #endif diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 8d3dcf4d4c1..6de2cab6b26 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -116,6 +116,7 @@ typedef struct Query bool hasAggs; /* has aggregates in tlist or havingQual */ bool hasWindowFuncs; /* has window functions in tlist */ + bool hasTargetSRFs; /* has set-returning functions in tlist */ bool hasSubLinks; /* has subquery SubLink */ bool hasDistinctOn; /* distinctClause is from DISTINCT ON */ bool hasRecursive; /* WITH RECURSIVE was specified */ diff --git a/src/include/parser/parse_func.h b/src/include/parser/parse_func.h index 0cefdf1b606..ed16d369829 100644 --- a/src/include/parser/parse_func.h +++ b/src/include/parser/parse_func.h @@ -67,4 +67,6 @@ extern Oid LookupFuncNameTypeNames(List *funcname, List *argtypes, extern Oid LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError); +extern void check_srf_call_placement(ParseState *pstate, int location); + #endif /* PARSE_FUNC_H */ diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index e3e359c0219..66335863db0 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -27,7 +27,7 @@ * by extension code that might need to call transformExpr(). The core code * will not enforce any context-driven restrictions on EXPR_KIND_OTHER * expressions, so the caller would have to check for sub-selects, aggregates, - * and window functions if those need to be disallowed. + * window functions, SRFs, etc if those need to be disallowed. */ typedef enum ParseExprKind { @@ -150,6 +150,7 @@ struct ParseState Node *p_value_substitute; /* what to replace VALUE with, if any */ bool p_hasAggs; bool p_hasWindowFuncs; + bool p_hasTargetSRFs; bool p_hasSubLinks; bool p_hasModifyingCTE; bool p_is_insert; diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 6141b7ab492..470cf935df3 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -6799,6 +6799,7 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) */ if (query->hasAggs || query->hasWindowFuncs || + query->hasTargetSRFs || query->hasSubLinks || query->hasForUpdate || query->cteList || diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out index 805e8db9f65..622f75517a0 100644 --- a/src/test/regress/expected/tsrf.out +++ b/src/test/regress/expected/tsrf.out @@ -359,15 +359,20 @@ SELECT * FROM fewmore; 5 (5 rows) --- nonsense that seems to be allowed +-- SRFs are not allowed in UPDATE (they once were, but it was nonsense) UPDATE fewmore SET data = generate_series(4,9); +ERROR: set-returning functions are not allowed in UPDATE +LINE 1: UPDATE fewmore SET data = generate_series(4,9); + ^ -- SRFs are not allowed in RETURNING INSERT INTO fewmore VALUES(1) RETURNING generate_series(1,3); -ERROR: set-valued function called in context that cannot accept a set +ERROR: set-returning functions are not allowed in RETURNING +LINE 1: INSERT INTO fewmore VALUES(1) RETURNING generate_series(1,3)... + ^ -- nor aggregate arguments SELECT count(generate_series(1,3)) FROM few; ERROR: set-valued function called in context that cannot accept a set --- nor proper VALUES +-- nor standalone VALUES (but surely this is a bug?) VALUES(1, generate_series(1,2)); ERROR: set-valued function called in context that cannot accept a set -- DISTINCT ON is evaluated before tSRF evaluation if SRF is not @@ -457,7 +462,7 @@ SELECT a, generate_series(1,2) FROM (VALUES(1),(2),(3)) r(a) LIMIT 2 OFFSET 2; -- SRFs are not allowed in LIMIT. SELECT 1 LIMIT generate_series(1,3); -ERROR: argument of LIMIT must not return a set +ERROR: set-returning functions are not allowed in LIMIT LINE 1: SELECT 1 LIMIT generate_series(1,3); ^ -- tSRF in correlated subquery, referencing table outside diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql index 524779581da..c28dd017e57 100644 --- a/src/test/regress/sql/tsrf.sql +++ b/src/test/regress/sql/tsrf.sql @@ -68,14 +68,14 @@ CREATE TABLE fewmore AS SELECT generate_series(1,3) AS data; INSERT INTO fewmore VALUES(generate_series(4,5)); SELECT * FROM fewmore; --- nonsense that seems to be allowed +-- SRFs are not allowed in UPDATE (they once were, but it was nonsense) UPDATE fewmore SET data = generate_series(4,9); -- SRFs are not allowed in RETURNING INSERT INTO fewmore VALUES(1) RETURNING generate_series(1,3); -- nor aggregate arguments SELECT count(generate_series(1,3)) FROM few; --- nor proper VALUES +-- nor standalone VALUES (but surely this is a bug?) VALUES(1, generate_series(1,2)); -- DISTINCT ON is evaluated before tSRF evaluation if SRF is not |