Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commita4c35ea

Browse files
committed
Improve parser's and planner's handling of set-returning functions.
Teach the parser to reject misplaced set-returning functions during parseanalysis using p_expr_kind, in much the same way as we do for aggregatesand window functions (cf commiteaccfde). While this isn't complete(it misses nesting-based restrictions), it's much better than the previouserror reporting for such cases, and it allows elimination of assortedad-hoc expression_returns_set() error checks. We could add nesting checkslater if it seems important to catch all cases at parse time.There is one case the parser will now throw error for although previousversions allowed it, which is SRFs in the tlist of an UPDATE. That neverbehaved sensibly (since it's ill-defined which generated row should beused to perform the update) and it's hard to see why it should not betreated as an error. It's a release-note-worthy change though.Also, add a new Query field hasTargetSRFs reporting whether there areany 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_setsearches. (There will be more such checks soon.) In some places, thisallows decontorting the logic since it's no longer expensive to check forSRFs in the tlist --- so I made the checks parallel to the handling ofhasAggs/hasWindowFuncs wherever it seemed appropriate.catversion bump because adding a Query field changes stored rules.Andres Freund and Tom LaneDiscussion: <24639.1473782855@sss.pgh.pa.us>
1 parent445a38a commita4c35ea

File tree

23 files changed

+225
-73
lines changed

23 files changed

+225
-73
lines changed

‎src/backend/catalog/heap.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2560,14 +2560,9 @@ cookDefault(ParseState *pstate,
25602560

25612561
/*
25622562
* transformExpr() should have already rejected subqueries, aggregates,
2563-
* and window functions, based on the EXPR_KIND_ for a default expression.
2564-
*
2565-
* It can't return a set either.
2563+
* window functions, and SRFs, based on the EXPR_KIND_ for a default
2564+
* expression.
25662565
*/
2567-
if (expression_returns_set(expr))
2568-
ereport(ERROR,
2569-
(errcode(ERRCODE_DATATYPE_MISMATCH),
2570-
errmsg("default expression must not return a set")));
25712566

25722567
/*
25732568
* Coerce the expression to the correct type and typmod, if given. This

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2731,6 +2731,7 @@ _copyQuery(const Query *from)
27312731
COPY_SCALAR_FIELD(resultRelation);
27322732
COPY_SCALAR_FIELD(hasAggs);
27332733
COPY_SCALAR_FIELD(hasWindowFuncs);
2734+
COPY_SCALAR_FIELD(hasTargetSRFs);
27342735
COPY_SCALAR_FIELD(hasSubLinks);
27352736
COPY_SCALAR_FIELD(hasDistinctOn);
27362737
COPY_SCALAR_FIELD(hasRecursive);

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,7 @@ _equalQuery(const Query *a, const Query *b)
921921
COMPARE_SCALAR_FIELD(resultRelation);
922922
COMPARE_SCALAR_FIELD(hasAggs);
923923
COMPARE_SCALAR_FIELD(hasWindowFuncs);
924+
COMPARE_SCALAR_FIELD(hasTargetSRFs);
924925
COMPARE_SCALAR_FIELD(hasSubLinks);
925926
COMPARE_SCALAR_FIELD(hasDistinctOn);
926927
COMPARE_SCALAR_FIELD(hasRecursive);

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2683,6 +2683,7 @@ _outQuery(StringInfo str, const Query *node)
26832683
WRITE_INT_FIELD(resultRelation);
26842684
WRITE_BOOL_FIELD(hasAggs);
26852685
WRITE_BOOL_FIELD(hasWindowFuncs);
2686+
WRITE_BOOL_FIELD(hasTargetSRFs);
26862687
WRITE_BOOL_FIELD(hasSubLinks);
26872688
WRITE_BOOL_FIELD(hasDistinctOn);
26882689
WRITE_BOOL_FIELD(hasRecursive);

‎src/backend/nodes/readfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ _readQuery(void)
238238
READ_INT_FIELD(resultRelation);
239239
READ_BOOL_FIELD(hasAggs);
240240
READ_BOOL_FIELD(hasWindowFuncs);
241+
READ_BOOL_FIELD(hasTargetSRFs);
241242
READ_BOOL_FIELD(hasSubLinks);
242243
READ_BOOL_FIELD(hasDistinctOn);
243244
READ_BOOL_FIELD(hasRecursive);

‎src/backend/optimizer/path/allpaths.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,7 +2422,8 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
24222422
continue;
24232423

24242424
/* Functions returning sets are unsafe (point 1) */
2425-
if (expression_returns_set((Node*)tle->expr))
2425+
if (subquery->hasTargetSRFs&&
2426+
expression_returns_set((Node*)tle->expr))
24262427
{
24272428
safetyInfo->unsafeColumns[tle->resno]= true;
24282429
continue;
@@ -2835,7 +2836,8 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel)
28352836
* If it contains a set-returning function, we can't remove it since
28362837
* that could change the number of rows returned by the subquery.
28372838
*/
2838-
if (expression_returns_set(texpr))
2839+
if (subquery->hasTargetSRFs&&
2840+
expression_returns_set(texpr))
28392841
continue;
28402842

28412843
/*

‎src/backend/optimizer/plan/analyzejoins.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,11 @@ rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, List *clause_list)
650650
bool
651651
query_supports_distinctness(Query*query)
652652
{
653+
/* we don't cope with SRFs, see comment below */
654+
if (query->hasTargetSRFs)
655+
return false;
656+
657+
/* check for features we can prove distinctness with */
653658
if (query->distinctClause!=NIL||
654659
query->groupClause!=NIL||
655660
query->groupingSets!=NIL||
@@ -695,7 +700,7 @@ query_is_distinct_for(Query *query, List *colnos, List *opids)
695700
* specified columns, since those must be evaluated before de-duplication;
696701
* but it doesn't presently seem worth the complication to check that.)
697702
*/
698-
if (expression_returns_set((Node*)query->targetList))
703+
if (query->hasTargetSRFs)
699704
return false;
700705

701706
/*

‎src/backend/optimizer/plan/planner.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,10 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
604604
preprocess_expression(root, (Node*)parse->targetList,
605605
EXPRKIND_TARGET);
606606

607+
/* Constant-folding might have removed all set-returning functions */
608+
if (parse->hasTargetSRFs)
609+
parse->hasTargetSRFs=expression_returns_set((Node*)parse->targetList);
610+
607611
newWithCheckOptions=NIL;
608612
foreach(l,parse->withCheckOptions)
609613
{
@@ -1702,16 +1706,14 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
17021706
* Figure out whether there's a hard limit on the number of rows that
17031707
* query_planner's result subplan needs to return. Even if we know a
17041708
* hard limit overall, it doesn't apply if the query has any
1705-
* grouping/aggregation operations. (XXX it also doesn't apply if the
1706-
* tlist contains any SRFs; but checking for that here seems more
1707-
* costly than it's worth, since root->limit_tuples is only used for
1708-
* cost estimates, and only in a small number of cases.)
1709+
* grouping/aggregation operations, or SRFs in the tlist.
17091710
*/
17101711
if (parse->groupClause||
17111712
parse->groupingSets||
17121713
parse->distinctClause||
17131714
parse->hasAggs||
17141715
parse->hasWindowFuncs||
1716+
parse->hasTargetSRFs||
17151717
root->hasHavingQual)
17161718
root->limit_tuples=-1.0;
17171719
else
@@ -1928,7 +1930,11 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
19281930
* weird usage that it doesn't seem worth greatly complicating matters to
19291931
* account for it.
19301932
*/
1931-
tlist_rows=tlist_returns_set_rows(tlist);
1933+
if (parse->hasTargetSRFs)
1934+
tlist_rows=tlist_returns_set_rows(tlist);
1935+
else
1936+
tlist_rows=1;
1937+
19321938
if (tlist_rows>1)
19331939
{
19341940
foreach(lc,current_rel->pathlist)
@@ -4995,7 +5001,8 @@ make_sort_input_target(PlannerInfo *root,
49955001
* Check for SRF or volatile functions. Check the SRF case first
49965002
* because we must know whether we have any postponed SRFs.
49975003
*/
4998-
if (expression_returns_set((Node*)expr))
5004+
if (parse->hasTargetSRFs&&
5005+
expression_returns_set((Node*)expr))
49995006
{
50005007
/* We'll decide below whether these are postponable */
50015008
col_is_srf[i]= true;
@@ -5034,6 +5041,7 @@ make_sort_input_target(PlannerInfo *root,
50345041
{
50355042
/* For sortgroupref cols, just check if any contain SRFs */
50365043
if (!have_srf_sortcols&&
5044+
parse->hasTargetSRFs&&
50375045
expression_returns_set((Node*)expr))
50385046
have_srf_sortcols= true;
50395047
}

‎src/backend/optimizer/plan/subselect.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,7 +1562,7 @@ simplify_EXISTS_query(PlannerInfo *root, Query *query)
15621562
{
15631563
/*
15641564
* We don't try to simplify at all if the query uses set operations,
1565-
* aggregates, grouping sets, modifying CTEs, HAVING, OFFSET, or FOR
1565+
* aggregates, grouping sets,SRFs,modifying CTEs, HAVING, OFFSET, or FOR
15661566
* UPDATE/SHARE; none of these seem likely in normal usage and their
15671567
* possible effects are complex. (Note: we could ignore an "OFFSET 0"
15681568
* clause, but that traditionally is used as an optimization fence, so we
@@ -1573,6 +1573,7 @@ simplify_EXISTS_query(PlannerInfo *root, Query *query)
15731573
query->hasAggs||
15741574
query->groupingSets||
15751575
query->hasWindowFuncs||
1576+
query->hasTargetSRFs||
15761577
query->hasModifyingCTE||
15771578
query->havingQual||
15781579
query->limitOffset||
@@ -1613,13 +1614,6 @@ simplify_EXISTS_query(PlannerInfo *root, Query *query)
16131614
query->limitCount=NULL;
16141615
}
16151616

1616-
/*
1617-
* Mustn't throw away the targetlist if it contains set-returning
1618-
* functions; those could affect whether zero rows are returned!
1619-
*/
1620-
if (expression_returns_set((Node*)query->targetList))
1621-
return false;
1622-
16231617
/*
16241618
* Otherwise, we can throw away the targetlist, as well as any GROUP,
16251619
* WINDOW, DISTINCT, and ORDER BY clauses; none of those clauses will

‎src/backend/optimizer/prep/prepjointree.c

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,8 +1188,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
11881188
parse->hasSubLinks |=subquery->hasSubLinks;
11891189

11901190
/*
1191-
* subquery won't be pulled up if it hasAggs orhasWindowFuncs,so no work
1192-
* needed on those flags
1191+
* subquery won't be pulled up if it hasAggs,hasWindowFuncs,or
1192+
*hasTargetSRFs, so no workneeded on those flags
11931193
*/
11941194

11951195
/*
@@ -1419,8 +1419,8 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte,
14191419
return false;
14201420

14211421
/*
1422-
* Can't pull up a subquery involving grouping, aggregation,sorting,
1423-
* limiting, or WITH. (XXX WITH could possibly be allowed later)
1422+
* Can't pull up a subquery involving grouping, aggregation,SRFs,
1423+
*sorting,limiting, or WITH. (XXX WITH could possibly be allowed later)
14241424
*
14251425
* We also don't pull up a subquery that has explicit FOR UPDATE/SHARE
14261426
* clauses, because pullup would cause the locking to occur semantically
@@ -1430,6 +1430,7 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte,
14301430
*/
14311431
if (subquery->hasAggs||
14321432
subquery->hasWindowFuncs||
1433+
subquery->hasTargetSRFs||
14331434
subquery->groupClause||
14341435
subquery->groupingSets||
14351436
subquery->havingQual||
@@ -1542,15 +1543,6 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte,
15421543
}
15431544
}
15441545

1545-
/*
1546-
* Don't pull up a subquery that has any set-returning functions in its
1547-
* targetlist. Otherwise we might well wind up inserting set-returning
1548-
* functions into places where they mustn't go, such as quals of higher
1549-
* queries. This also ensures deletion of an empty jointree is valid.
1550-
*/
1551-
if (expression_returns_set((Node*)subquery->targetList))
1552-
return false;
1553-
15541546
/*
15551547
* Don't pull up a subquery that has any volatile functions in its
15561548
* targetlist. Otherwise we might introduce multiple evaluations of these

‎src/backend/optimizer/util/clauses.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4449,6 +4449,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
44494449
querytree->utilityStmt||
44504450
querytree->hasAggs||
44514451
querytree->hasWindowFuncs||
4452+
querytree->hasTargetSRFs||
44524453
querytree->hasSubLinks||
44534454
querytree->cteList||
44544455
querytree->rtable||
@@ -4489,17 +4490,13 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
44894490
Assert(!modifyTargetList);
44904491

44914492
/*
4492-
* Additional validity checks on the expression. It mustn't return a set,
4493-
* and it mustn't be more volatile than the surrounding function (this is
4494-
* to avoid breaking hacks that involve pretending a function is immutable
4495-
* when it really ain't). If the surrounding function is declared strict,
4496-
* then the expression must contain only strict constructs and must use
4497-
* all of the function parameters (this is overkill, but an exact analysis
4498-
* is hard).
4493+
* Additional validity checks on the expression. It mustn't be more
4494+
* volatile than the surrounding function (this is to avoid breaking hacks
4495+
* that involve pretending a function is immutable when it really ain't).
4496+
* If the surrounding function is declared strict, then the expression
4497+
* must contain only strict constructs and must use all of the function
4498+
* parameters (this is overkill, but an exact analysis is hard).
44994499
*/
4500-
if (expression_returns_set(newexpr))
4501-
gotofail;
4502-
45034500
if (funcform->provolatile==PROVOLATILE_IMMUTABLE&&
45044501
contain_mutable_functions(newexpr))
45054502
gotofail;

‎src/backend/parser/analyze.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
417417

418418
qry->hasSubLinks=pstate->p_hasSubLinks;
419419
qry->hasWindowFuncs=pstate->p_hasWindowFuncs;
420+
qry->hasTargetSRFs=pstate->p_hasTargetSRFs;
420421
qry->hasAggs=pstate->p_hasAggs;
421422
if (pstate->p_hasAggs)
422423
parseCheckAggregates(pstate,qry);
@@ -819,6 +820,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
819820
qry->rtable=pstate->p_rtable;
820821
qry->jointree=makeFromExpr(pstate->p_joinlist,NULL);
821822

823+
qry->hasTargetSRFs=pstate->p_hasTargetSRFs;
822824
qry->hasSubLinks=pstate->p_hasSubLinks;
823825

824826
assign_query_collations(pstate,qry);
@@ -1231,6 +1233,7 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
12311233

12321234
qry->hasSubLinks=pstate->p_hasSubLinks;
12331235
qry->hasWindowFuncs=pstate->p_hasWindowFuncs;
1236+
qry->hasTargetSRFs=pstate->p_hasTargetSRFs;
12341237
qry->hasAggs=pstate->p_hasAggs;
12351238
if (pstate->p_hasAggs||qry->groupClause||qry->groupingSets||qry->havingQual)
12361239
parseCheckAggregates(pstate,qry);
@@ -1691,6 +1694,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
16911694

16921695
qry->hasSubLinks=pstate->p_hasSubLinks;
16931696
qry->hasWindowFuncs=pstate->p_hasWindowFuncs;
1697+
qry->hasTargetSRFs=pstate->p_hasTargetSRFs;
16941698
qry->hasAggs=pstate->p_hasAggs;
16951699
if (pstate->p_hasAggs||qry->groupClause||qry->groupingSets||qry->havingQual)
16961700
parseCheckAggregates(pstate,qry);
@@ -2170,6 +2174,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
21702174
qry->rtable=pstate->p_rtable;
21712175
qry->jointree=makeFromExpr(pstate->p_joinlist,qual);
21722176

2177+
qry->hasTargetSRFs=pstate->p_hasTargetSRFs;
21732178
qry->hasSubLinks=pstate->p_hasSubLinks;
21742179

21752180
assign_query_collations(pstate,qry);
@@ -2565,7 +2570,7 @@ CheckSelectLocking(Query *qry, LockClauseStrength strength)
25652570
translator: %s is a SQL row locking clause such as FOR UPDATE */
25662571
errmsg("%s is not allowed with window functions",
25672572
LCS_asString(strength))));
2568-
if (expression_returns_set((Node*)qry->targetList))
2573+
if (qry->hasTargetSRFs)
25692574
ereport(ERROR,
25702575
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
25712576
/*------

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp