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

Commit371e3da

Browse files
committed
Fix incorrect logic for determining safe WindowAgg run conditions
The logic added in9d9c02c to determine when a qual can be used as aWindowClause run condition failed to correctly check for subqueries in thequal. This was being done correctly for normal subquery qual pushdowns,it's just that9d9c02c failed to follow the lead on that.This also fixes various other cases where transforming the qual into aWindowClause run condition in the subquery should have been disallowed.Bug: #17826Reported-by: Anban CompanyDiscussion:https://postgr.es/m/17826-7d8750952f19a5f5@postgresql.orgBackpatch-through: 15, where9d9c02c was introduced.
1 parentfd65711 commit371e3da

File tree

3 files changed

+161
-84
lines changed

3 files changed

+161
-84
lines changed

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

Lines changed: 119 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,32 @@
5151
#include"utils/lsyscache.h"
5252

5353

54+
/* Bitmask flags for pushdown_safety_info.unsafeFlags */
55+
#defineUNSAFE_HAS_VOLATILE_FUNC(1 << 0)
56+
#defineUNSAFE_HAS_SET_FUNC(1 << 1)
57+
#defineUNSAFE_NOTIN_DISTINCTON_CLAUSE(1 << 2)
58+
#defineUNSAFE_NOTIN_PARTITIONBY_CLAUSE(1 << 3)
59+
#defineUNSAFE_TYPE_MISMATCH(1 << 4)
60+
5461
/* results of subquery_is_pushdown_safe */
5562
typedefstructpushdown_safety_info
5663
{
57-
bool*unsafeColumns;/* which output columns are unsafe to use */
64+
unsignedchar*unsafeFlags;/* bitmask of reasons why this target list
65+
* column is unsafe for qual pushdown, or 0 if
66+
* no reason. */
5867
boolunsafeVolatile;/* don't push down volatile quals */
5968
boolunsafeLeaky;/* don't push down leaky quals */
6069
}pushdown_safety_info;
6170

71+
/* Return type for qual_is_pushdown_safe */
72+
typedefenumpushdown_safe_type
73+
{
74+
PUSHDOWN_UNSAFE,/* unsafe to push qual into subquery */
75+
PUSHDOWN_SAFE,/* safe to push qual into subquery */
76+
PUSHDOWN_WINDOWCLAUSE_RUNCOND/* unsafe, but may work as WindowClause
77+
* run condition */
78+
}pushdown_safe_type;
79+
6280
/* These parameters are set by GUC */
6381
boolenable_geqo= false;/* just in case GUC doesn't set it */
6482
intgeqo_threshold;
@@ -135,9 +153,9 @@ static void check_output_expressions(Query *subquery,
135153
staticvoidcompare_tlist_datatypes(List*tlist,List*colTypes,
136154
pushdown_safety_info*safetyInfo);
137155
staticbooltargetIsInAllPartitionLists(TargetEntry*tle,Query*query);
138-
staticboolqual_is_pushdown_safe(Query*subquery,Indexrti,
139-
RestrictInfo*rinfo,
140-
pushdown_safety_info*safetyInfo);
156+
staticpushdown_safe_typequal_is_pushdown_safe(Query*subquery,Indexrti,
157+
RestrictInfo*rinfo,
158+
pushdown_safety_info*safetyInfo);
141159
staticvoidsubquery_push_qual(Query*subquery,
142160
RangeTblEntry*rte,Indexrti,Node*qual);
143161
staticvoidrecurse_push_qual(Node*setOp,Query*topquery,
@@ -2207,6 +2225,10 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
22072225
if (!IsA(wfunc,WindowFunc))
22082226
return false;
22092227

2228+
/* can't use it if there are subplans in the WindowFunc */
2229+
if (contain_subplans((Node*)wfunc))
2230+
return false;
2231+
22102232
prosupport=get_func_support(wfunc->winfnoid);
22112233

22122234
/* Check if there's a support function for 'wfunc' */
@@ -2488,13 +2510,14 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
24882510
/*
24892511
* Zero out result area for subquery_is_pushdown_safe, so that it can set
24902512
* flags as needed while recursing. In particular, we need a workspace
2491-
* for keeping track of unsafe-to-reference columns. unsafeColumns[i]
2492-
* will be set true if we find that output column i of the subquery is
2493-
* unsafe to use in a pushed-down qual.
2513+
* for keeping track of the reasons why columns are unsafe to reference.
2514+
* These reasons are stored in the bits inside unsafeFlags[i] when we
2515+
* discover reasons that column i of the subquery is unsafe to be used in
2516+
* a pushed-down qual.
24942517
*/
24952518
memset(&safetyInfo,0,sizeof(safetyInfo));
2496-
safetyInfo.unsafeColumns= (bool*)
2497-
palloc0((list_length(subquery->targetList)+1)*sizeof(bool));
2519+
safetyInfo.unsafeFlags= (unsignedchar*)
2520+
palloc0((list_length(subquery->targetList)+1)*sizeof(unsignedchar));
24982521

24992522
/*
25002523
* If the subquery has the "security_barrier" flag, it means the subquery
@@ -2537,37 +2560,50 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
25372560
RestrictInfo*rinfo= (RestrictInfo*)lfirst(l);
25382561
Node*clause= (Node*)rinfo->clause;
25392562

2540-
if (!rinfo->pseudoconstant&&
2541-
qual_is_pushdown_safe(subquery,rti,rinfo,&safetyInfo))
2563+
if (rinfo->pseudoconstant)
25422564
{
2543-
/* Push it down */
2544-
subquery_push_qual(subquery,rte,rti,clause);
2565+
upperrestrictlist=lappend(upperrestrictlist,rinfo);
2566+
continue;
25452567
}
2546-
else
2568+
2569+
switch (qual_is_pushdown_safe(subquery,rti,rinfo,&safetyInfo))
25472570
{
2548-
/*
2549-
* Since we can't push the qual down into the subquery, check
2550-
* if it happens to reference a window function. If so then
2551-
* it might be useful to use for the WindowAgg's runCondition.
2552-
*/
2553-
if (!subquery->hasWindowFuncs||
2554-
check_and_push_window_quals(subquery,rte,rti,clause,
2555-
&run_cond_attrs))
2556-
{
2571+
casePUSHDOWN_SAFE:
2572+
/* Push it down */
2573+
subquery_push_qual(subquery,rte,rti,clause);
2574+
break;
2575+
2576+
casePUSHDOWN_WINDOWCLAUSE_RUNCOND:
2577+
25572578
/*
2558-
* subquery has no window funcs or the clause is not a
2559-
* suitable window run condition qual or it is, but the
2560-
* original must also be kept in the upper query.
2579+
* Since we can't push the qual down into the subquery,
2580+
* check if it happens to reference a window function. If
2581+
* so then it might be useful to use for the WindowAgg's
2582+
* runCondition.
25612583
*/
2584+
if (!subquery->hasWindowFuncs||
2585+
check_and_push_window_quals(subquery,rte,rti,clause,
2586+
&run_cond_attrs))
2587+
{
2588+
/*
2589+
* subquery has no window funcs or the clause is not a
2590+
* suitable window run condition qual or it is, but
2591+
* the original must also be kept in the upper query.
2592+
*/
2593+
upperrestrictlist=lappend(upperrestrictlist,rinfo);
2594+
}
2595+
break;
2596+
2597+
casePUSHDOWN_UNSAFE:
25622598
upperrestrictlist=lappend(upperrestrictlist,rinfo);
2563-
}
2599+
break;
25642600
}
25652601
}
25662602
rel->baserestrictinfo=upperrestrictlist;
25672603
/* We don't bother recomputing baserestrict_min_security */
25682604
}
25692605

2570-
pfree(safetyInfo.unsafeColumns);
2606+
pfree(safetyInfo.unsafeFlags);
25712607

25722608
/*
25732609
* The upper query might not use all the subquery's output columns; if
@@ -3492,13 +3528,13 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
34923528
*
34933529
* In addition, we make several checks on the subquery's output columns to see
34943530
* if it is safe to reference them in pushed-down quals. If output column k
3495-
* is found to be unsafe to reference, we setsafetyInfo->unsafeColumns[k]
3496-
*to true, but we don't reject the subquery overall since column k might not
3497-
* be referenced by some/all quals. TheunsafeColumns[] array will be
3498-
* consulted later by qual_is_pushdown_safe(). It's better to do it this way
3499-
* than to make the checks directly in qual_is_pushdown_safe(), because when
3500-
* the subquery involves set operations we have to check the output
3501-
* expressions in each arm of the set op.
3531+
* is found to be unsafe to reference, we setthe reason for that inside
3532+
*safetyInfo->unsafeFlags[k], but we don't reject the subquery overall since
3533+
*column k might notbe referenced by some/all quals. TheunsafeFlags[]
3534+
*array will beconsulted later by qual_is_pushdown_safe(). It's better to
3535+
*do it this waythan to make the checks directly in qual_is_pushdown_safe(),
3536+
*because whenthe subquery involves set operations we have to check the
3537+
*outputexpressions in each arm of the set op.
35023538
*
35033539
* Note: pushing quals into a DISTINCT subquery is theoretically dubious:
35043540
* we're effectively assuming that the quals cannot distinguish values that
@@ -3546,9 +3582,9 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
35463582

35473583
/*
35483584
* If we're at a leaf query, check for unsafe expressions in its target
3549-
* list, and mark anyunsafe onesinunsafeColumns[]. (Non-leaf nodes in
3550-
* setop trees have only simple Vars in their tlists, so no need to check
3551-
* them.)
3585+
* list, and mark anyreasons why they're unsafeinunsafeFlags[].
3586+
*(Non-leaf nodes insetop trees have only simple Vars in their tlists,
3587+
*so no need to checkthem.)
35523588
*/
35533589
if (subquery->setOperations==NULL)
35543590
check_output_expressions(subquery,safetyInfo);
@@ -3619,9 +3655,9 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
36193655
*
36203656
* There are several cases in which it's unsafe to push down an upper-level
36213657
* qual if it references a particular output column of a subquery. We check
3622-
* each output column of the subquery and setunsafeColumns[k]to true if
3623-
* that column is unsafe for a pushed-down qual to reference. The conditions
3624-
* checked here are:
3658+
* each output column of the subquery and setflags in unsafeFlags[k]when we
3659+
*seethat column is unsafe for a pushed-down qual to reference. The
3660+
*conditionschecked here are:
36253661
*
36263662
* 1. We must not push down any quals that refer to subselect outputs that
36273663
* return sets, else we'd introduce functions-returning-sets into the
@@ -3645,7 +3681,9 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
36453681
* every row of any one window partition, and totally excluding some
36463682
* partitions will not change a window function's results for remaining
36473683
* partitions. (Again, this also requires nonvolatile quals, but
3648-
* subquery_is_pushdown_safe handles that.)
3684+
* subquery_is_pushdown_safe handles that.). Subquery columns marked as
3685+
* unsafe for this reason can still have WindowClause run conditions pushed
3686+
* down.
36493687
*/
36503688
staticvoid
36513689
check_output_expressions(Query*subquery,pushdown_safety_info*safetyInfo)
@@ -3659,40 +3697,44 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
36593697
if (tle->resjunk)
36603698
continue;/* ignore resjunk columns */
36613699

3662-
/* We need not check further if output col is already known unsafe */
3663-
if (safetyInfo->unsafeColumns[tle->resno])
3664-
continue;
3665-
36663700
/* Functions returning sets are unsafe (point 1) */
36673701
if (subquery->hasTargetSRFs&&
3702+
(safetyInfo->unsafeFlags[tle->resno]&
3703+
UNSAFE_HAS_SET_FUNC)==0&&
36683704
expression_returns_set((Node*)tle->expr))
36693705
{
3670-
safetyInfo->unsafeColumns[tle->resno]= true;
3706+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_HAS_SET_FUNC;
36713707
continue;
36723708
}
36733709

36743710
/* Volatile functions are unsafe (point 2) */
3675-
if (contain_volatile_functions((Node*)tle->expr))
3711+
if ((safetyInfo->unsafeFlags[tle->resno]&
3712+
UNSAFE_HAS_VOLATILE_FUNC)==0&&
3713+
contain_volatile_functions((Node*)tle->expr))
36763714
{
3677-
safetyInfo->unsafeColumns[tle->resno]= true;
3715+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_HAS_VOLATILE_FUNC;
36783716
continue;
36793717
}
36803718

36813719
/* If subquery uses DISTINCT ON, check point 3 */
36823720
if (subquery->hasDistinctOn&&
3721+
(safetyInfo->unsafeFlags[tle->resno]&
3722+
UNSAFE_NOTIN_DISTINCTON_CLAUSE)==0&&
36833723
!targetIsInSortList(tle,InvalidOid,subquery->distinctClause))
36843724
{
36853725
/* non-DISTINCT column, so mark it unsafe */
3686-
safetyInfo->unsafeColumns[tle->resno]= true;
3726+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_NOTIN_DISTINCTON_CLAUSE;
36873727
continue;
36883728
}
36893729

36903730
/* If subquery uses window functions, check point 4 */
36913731
if (subquery->hasWindowFuncs&&
3732+
(safetyInfo->unsafeFlags[tle->resno]&
3733+
UNSAFE_NOTIN_DISTINCTON_CLAUSE)==0&&
36923734
!targetIsInAllPartitionLists(tle,subquery))
36933735
{
36943736
/* not present in all PARTITION BY clauses, so mark it unsafe */
3695-
safetyInfo->unsafeColumns[tle->resno]= true;
3737+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_NOTIN_PARTITIONBY_CLAUSE;
36963738
continue;
36973739
}
36983740
}
@@ -3704,16 +3746,16 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
37043746
* subquery columns that suffer no type coercions in the set operation.
37053747
* Otherwise there are possible semantic gotchas. So, we check the
37063748
* component queries to see if any of them have output types different from
3707-
* the top-level setop outputs.unsafeColumns[k] issettrue if column k
3708-
* has different type in any component.
3749+
* the top-level setop outputs.Wesetthe UNSAFE_TYPE_MISMATCH bit in
3750+
*unsafeFlags[k] if column khas different type in any component.
37093751
*
37103752
* We don't have to care about typmods here: the only allowed difference
37113753
* between set-op input and output typmods is input is a specific typmod
37123754
* and output is -1, and that does not require a coercion.
37133755
*
37143756
* tlist is a subquery tlist.
37153757
* colTypes is an OID list of the top-level setop's output column types.
3716-
* safetyInfo->unsafeColumns[] is theresult array.
3758+
* safetyInfo is thepushdown_safety_info to set unsafeFlags[] for.
37173759
*/
37183760
staticvoid
37193761
compare_tlist_datatypes(List*tlist,List*colTypes,
@@ -3731,7 +3773,7 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
37313773
if (colType==NULL)
37323774
elog(ERROR,"wrong number of tlist entries");
37333775
if (exprType((Node*)tle->expr)!=lfirst_oid(colType))
3734-
safetyInfo->unsafeColumns[tle->resno]= true;
3776+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_TYPE_MISMATCH;
37353777
colType=lnext(colTypes,colType);
37363778
}
37373779
if (colType!=NULL)
@@ -3791,28 +3833,28 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query)
37913833
* 5. rinfo's clause must not refer to any subquery output columns that were
37923834
* found to be unsafe to reference by subquery_is_pushdown_safe().
37933835
*/
3794-
staticbool
3836+
staticpushdown_safe_type
37953837
qual_is_pushdown_safe(Query*subquery,Indexrti,RestrictInfo*rinfo,
37963838
pushdown_safety_info*safetyInfo)
37973839
{
3798-
boolsafe=true;
3840+
pushdown_safe_typesafe=PUSHDOWN_SAFE;
37993841
Node*qual= (Node*)rinfo->clause;
38003842
List*vars;
38013843
ListCell*vl;
38023844

38033845
/* Refuse subselects (point 1) */
38043846
if (contain_subplans(qual))
3805-
returnfalse;
3847+
returnPUSHDOWN_UNSAFE;
38063848

38073849
/* Refuse volatile quals if we found they'd be unsafe (point 2) */
38083850
if (safetyInfo->unsafeVolatile&&
38093851
contain_volatile_functions((Node*)rinfo))
3810-
returnfalse;
3852+
returnPUSHDOWN_UNSAFE;
38113853

38123854
/* Refuse leaky quals if told to (point 3) */
38133855
if (safetyInfo->unsafeLeaky&&
38143856
contain_leaked_vars(qual))
3815-
returnfalse;
3857+
returnPUSHDOWN_UNSAFE;
38163858

38173859
/*
38183860
* It would be unsafe to push down window function calls, but at least for
@@ -3841,7 +3883,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
38413883
*/
38423884
if (!IsA(var,Var))
38433885
{
3844-
safe=false;
3886+
safe=PUSHDOWN_UNSAFE;
38453887
break;
38463888
}
38473889

@@ -3853,7 +3895,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
38533895
*/
38543896
if (var->varno!=rti)
38553897
{
3856-
safe=false;
3898+
safe=PUSHDOWN_UNSAFE;
38573899
break;
38583900
}
38593901

@@ -3863,15 +3905,26 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
38633905
/* Check point 4 */
38643906
if (var->varattno==0)
38653907
{
3866-
safe=false;
3908+
safe=PUSHDOWN_UNSAFE;
38673909
break;
38683910
}
38693911

38703912
/* Check point 5 */
3871-
if (safetyInfo->unsafeColumns[var->varattno])
3913+
if (safetyInfo->unsafeFlags[var->varattno]!=0)
38723914
{
3873-
safe= false;
3874-
break;
3915+
if (safetyInfo->unsafeFlags[var->varattno]&
3916+
(UNSAFE_HAS_VOLATILE_FUNC |UNSAFE_HAS_SET_FUNC |
3917+
UNSAFE_NOTIN_DISTINCTON_CLAUSE |UNSAFE_TYPE_MISMATCH))
3918+
{
3919+
safe=PUSHDOWN_UNSAFE;
3920+
break;
3921+
}
3922+
else
3923+
{
3924+
/* UNSAFE_NOTIN_PARTITIONBY_CLAUSE is ok for run conditions */
3925+
safe=PUSHDOWN_WINDOWCLAUSE_RUNCOND;
3926+
/* don't break, we might find another Var that's unsafe */
3927+
}
38753928
}
38763929
}
38773930

0 commit comments

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