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

Commitda4ed75

Browse files
committed
Fix incorrect tests for SRFs in relation_can_be_sorted_early().
Commitfac1b47 thought we could check for set-returning functionsby testing only the top-level node in an expression tree. This iswrong in itself, and to make matters worse it encouraged othersto make the same mistake, by exporting tlist.c's special-purposeIS_SRF_CALL() as a widely-visible macro. I can't find any evidencethat anyone's taken the bait, but it was only a matter of time.Use expression_returns_set() instead, and stuff the IS_SRF_CALL()genie back in its bottle, this time with a warning label. I alsoadded a couple of cross-reference comments.After a fair amount of fooling around, I've despaired of makinga robust test case that exposes the bug reliably, so no test casehere. (Note that the test case added byfac1b47 is itselfbroken, in that it doesn't notice if you remove the code change.The repro given by the bug submitter currently doesn't fail eitherin v15 or HEAD, though I suspect that may indicate an unrelated bug.)Per bug #17564 from Martijn van Oosterhout. Back-patch to v13,as the faulty patch was.Discussion:https://postgr.es/m/17564-c7472c2f90ef2da3@postgresql.org
1 parentb2694ae commitda4ed75

File tree

4 files changed

+19
-7
lines changed

4 files changed

+19
-7
lines changed

‎src/backend/nodes/nodeFuncs.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,12 @@ expression_returns_set_walker(Node *node, void *context)
742742
/* else fall through to check args */
743743
}
744744

745+
/*
746+
* If you add any more cases that return sets, also fix
747+
* expression_returns_set_rows() in clauses.c and IS_SRF_CALL() in
748+
* tlist.c.
749+
*/
750+
745751
/* Avoid recursion for some cases that parser checks not to return a set */
746752
if (IsA(node,Aggref))
747753
return false;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,7 +1002,7 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec,
10021002
* one are effectively checking properties of targetexpr, so there's
10031003
* no point in asking whether some other EC member would be better.)
10041004
*/
1005-
if (IS_SRF_CALL((Node*)em->em_expr))
1005+
if (expression_returns_set((Node*)em->em_expr))
10061006
continue;
10071007

10081008
/*
@@ -1030,7 +1030,7 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec,
10301030
* member in this case; since SRFs can't appear in WHERE, they cannot
10311031
* belong to multi-member ECs.)
10321032
*/
1033-
if (IS_SRF_CALL((Node*)em->em_expr))
1033+
if (expression_returns_set((Node*)em->em_expr))
10341034
returnNULL;
10351035

10361036
returnem->em_expr;

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,17 @@
2121
#include"optimizer/tlist.h"
2222

2323

24+
/*
25+
* Test if an expression node represents a SRF call. Beware multiple eval!
26+
*
27+
* Please note that this is only meant for use in split_pathtarget_at_srfs();
28+
* if you use it anywhere else, your code is almost certainly wrong for SRFs
29+
* nested within expressions. Use expression_returns_set() instead.
30+
*/
31+
#defineIS_SRF_CALL(node) \
32+
((IsA(node, FuncExpr) && ((FuncExpr *) (node))->funcretset) || \
33+
(IsA(node, OpExpr) && ((OpExpr *) (node))->opretset))
34+
2435
/*
2536
* Data structures for split_pathtarget_at_srfs(). To preserve the identity
2637
* of sortgroupref items even if they are textually equal(), what we track is

‎src/include/optimizer/optimizer.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,6 @@
2424

2525
#include"nodes/parsenodes.h"
2626

27-
/* Test if an expression node represents a SRF call. Beware multiple eval! */
28-
#defineIS_SRF_CALL(node) \
29-
((IsA(node, FuncExpr) && ((FuncExpr *) (node))->funcretset) || \
30-
(IsA(node, OpExpr) && ((OpExpr *) (node))->opretset))
31-
3227
/*
3328
* We don't want to include nodes/pathnodes.h here, because non-planner
3429
* code should generally treat PlannerInfo as an opaque typedef.

0 commit comments

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