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

Commiteb7d043

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 parentb9f8d1c commiteb7d043

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
@@ -52,14 +52,32 @@
5252
#include"utils/lsyscache.h"
5353

5454

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

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

2239+
/* can't use it if there are subplans in the WindowFunc */
2240+
if (contain_subplans((Node*)wfunc))
2241+
return false;
2242+
22212243
prosupport=get_func_support(wfunc->winfnoid);
22222244

22232245
/* Check if there's a support function for 'wfunc' */
@@ -2500,13 +2522,14 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
25002522
/*
25012523
* Zero out result area for subquery_is_pushdown_safe, so that it can set
25022524
* flags as needed while recursing. In particular, we need a workspace
2503-
* for keeping track of unsafe-to-reference columns. unsafeColumns[i]
2504-
* will be set true if we find that output column i of the subquery is
2505-
* unsafe to use in a pushed-down qual.
2525+
* for keeping track of the reasons why columns are unsafe to reference.
2526+
* These reasons are stored in the bits inside unsafeFlags[i] when we
2527+
* discover reasons that column i of the subquery is unsafe to be used in
2528+
* a pushed-down qual.
25062529
*/
25072530
memset(&safetyInfo,0,sizeof(safetyInfo));
2508-
safetyInfo.unsafeColumns= (bool*)
2509-
palloc0((list_length(subquery->targetList)+1)*sizeof(bool));
2531+
safetyInfo.unsafeFlags= (unsignedchar*)
2532+
palloc0((list_length(subquery->targetList)+1)*sizeof(unsignedchar));
25102533

25112534
/*
25122535
* If the subquery has the "security_barrier" flag, it means the subquery
@@ -2549,37 +2572,50 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
25492572
RestrictInfo*rinfo= (RestrictInfo*)lfirst(l);
25502573
Node*clause= (Node*)rinfo->clause;
25512574

2552-
if (!rinfo->pseudoconstant&&
2553-
qual_is_pushdown_safe(subquery,rti,rinfo,&safetyInfo))
2575+
if (rinfo->pseudoconstant)
25542576
{
2555-
/* Push it down */
2556-
subquery_push_qual(subquery,rte,rti,clause);
2577+
upperrestrictlist=lappend(upperrestrictlist,rinfo);
2578+
continue;
25572579
}
2558-
else
2580+
2581+
switch (qual_is_pushdown_safe(subquery,rti,rinfo,&safetyInfo))
25592582
{
2560-
/*
2561-
* Since we can't push the qual down into the subquery, check
2562-
* if it happens to reference a window function. If so then
2563-
* it might be useful to use for the WindowAgg's runCondition.
2564-
*/
2565-
if (!subquery->hasWindowFuncs||
2566-
check_and_push_window_quals(subquery,rte,rti,clause,
2567-
&run_cond_attrs))
2568-
{
2583+
casePUSHDOWN_SAFE:
2584+
/* Push it down */
2585+
subquery_push_qual(subquery,rte,rti,clause);
2586+
break;
2587+
2588+
casePUSHDOWN_WINDOWCLAUSE_RUNCOND:
2589+
25692590
/*
2570-
* subquery has no window funcs or the clause is not a
2571-
* suitable window run condition qual or it is, but the
2572-
* original must also be kept in the upper query.
2591+
* Since we can't push the qual down into the subquery,
2592+
* check if it happens to reference a window function. If
2593+
* so then it might be useful to use for the WindowAgg's
2594+
* runCondition.
25732595
*/
2596+
if (!subquery->hasWindowFuncs||
2597+
check_and_push_window_quals(subquery,rte,rti,clause,
2598+
&run_cond_attrs))
2599+
{
2600+
/*
2601+
* subquery has no window funcs or the clause is not a
2602+
* suitable window run condition qual or it is, but
2603+
* the original must also be kept in the upper query.
2604+
*/
2605+
upperrestrictlist=lappend(upperrestrictlist,rinfo);
2606+
}
2607+
break;
2608+
2609+
casePUSHDOWN_UNSAFE:
25742610
upperrestrictlist=lappend(upperrestrictlist,rinfo);
2575-
}
2611+
break;
25762612
}
25772613
}
25782614
rel->baserestrictinfo=upperrestrictlist;
25792615
/* We don't bother recomputing baserestrict_min_security */
25802616
}
25812617

2582-
pfree(safetyInfo.unsafeColumns);
2618+
pfree(safetyInfo.unsafeFlags);
25832619

25842620
/*
25852621
* The upper query might not use all the subquery's output columns; if
@@ -3517,13 +3553,13 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
35173553
*
35183554
* In addition, we make several checks on the subquery's output columns to see
35193555
* if it is safe to reference them in pushed-down quals. If output column k
3520-
* is found to be unsafe to reference, we setsafetyInfo->unsafeColumns[k]
3521-
*to true, but we don't reject the subquery overall since column k might not
3522-
* be referenced by some/all quals. TheunsafeColumns[] array will be
3523-
* consulted later by qual_is_pushdown_safe(). It's better to do it this way
3524-
* than to make the checks directly in qual_is_pushdown_safe(), because when
3525-
* the subquery involves set operations we have to check the output
3526-
* expressions in each arm of the set op.
3556+
* is found to be unsafe to reference, we setthe reason for that inside
3557+
*safetyInfo->unsafeFlags[k], but we don't reject the subquery overall since
3558+
*column k might notbe referenced by some/all quals. TheunsafeFlags[]
3559+
*array will beconsulted later by qual_is_pushdown_safe(). It's better to
3560+
*do it this waythan to make the checks directly in qual_is_pushdown_safe(),
3561+
*because whenthe subquery involves set operations we have to check the
3562+
*outputexpressions in each arm of the set op.
35273563
*
35283564
* Note: pushing quals into a DISTINCT subquery is theoretically dubious:
35293565
* we're effectively assuming that the quals cannot distinguish values that
@@ -3571,9 +3607,9 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
35713607

35723608
/*
35733609
* If we're at a leaf query, check for unsafe expressions in its target
3574-
* list, and mark anyunsafe onesinunsafeColumns[]. (Non-leaf nodes in
3575-
* setop trees have only simple Vars in their tlists, so no need to check
3576-
* them.)
3610+
* list, and mark anyreasons why they're unsafeinunsafeFlags[].
3611+
*(Non-leaf nodes insetop trees have only simple Vars in their tlists,
3612+
*so no need to checkthem.)
35773613
*/
35783614
if (subquery->setOperations==NULL)
35793615
check_output_expressions(subquery,safetyInfo);
@@ -3644,9 +3680,9 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
36443680
*
36453681
* There are several cases in which it's unsafe to push down an upper-level
36463682
* qual if it references a particular output column of a subquery. We check
3647-
* each output column of the subquery and setunsafeColumns[k]to true if
3648-
* that column is unsafe for a pushed-down qual to reference. The conditions
3649-
* checked here are:
3683+
* each output column of the subquery and setflags in unsafeFlags[k]when we
3684+
*seethat column is unsafe for a pushed-down qual to reference. The
3685+
*conditionschecked here are:
36503686
*
36513687
* 1. We must not push down any quals that refer to subselect outputs that
36523688
* return sets, else we'd introduce functions-returning-sets into the
@@ -3670,7 +3706,9 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
36703706
* every row of any one window partition, and totally excluding some
36713707
* partitions will not change a window function's results for remaining
36723708
* partitions. (Again, this also requires nonvolatile quals, but
3673-
* subquery_is_pushdown_safe handles that.)
3709+
* subquery_is_pushdown_safe handles that.). Subquery columns marked as
3710+
* unsafe for this reason can still have WindowClause run conditions pushed
3711+
* down.
36743712
*/
36753713
staticvoid
36763714
check_output_expressions(Query*subquery,pushdown_safety_info*safetyInfo)
@@ -3684,40 +3722,44 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
36843722
if (tle->resjunk)
36853723
continue;/* ignore resjunk columns */
36863724

3687-
/* We need not check further if output col is already known unsafe */
3688-
if (safetyInfo->unsafeColumns[tle->resno])
3689-
continue;
3690-
36913725
/* Functions returning sets are unsafe (point 1) */
36923726
if (subquery->hasTargetSRFs&&
3727+
(safetyInfo->unsafeFlags[tle->resno]&
3728+
UNSAFE_HAS_SET_FUNC)==0&&
36933729
expression_returns_set((Node*)tle->expr))
36943730
{
3695-
safetyInfo->unsafeColumns[tle->resno]= true;
3731+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_HAS_SET_FUNC;
36963732
continue;
36973733
}
36983734

36993735
/* Volatile functions are unsafe (point 2) */
3700-
if (contain_volatile_functions((Node*)tle->expr))
3736+
if ((safetyInfo->unsafeFlags[tle->resno]&
3737+
UNSAFE_HAS_VOLATILE_FUNC)==0&&
3738+
contain_volatile_functions((Node*)tle->expr))
37013739
{
3702-
safetyInfo->unsafeColumns[tle->resno]= true;
3740+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_HAS_VOLATILE_FUNC;
37033741
continue;
37043742
}
37053743

37063744
/* If subquery uses DISTINCT ON, check point 3 */
37073745
if (subquery->hasDistinctOn&&
3746+
(safetyInfo->unsafeFlags[tle->resno]&
3747+
UNSAFE_NOTIN_DISTINCTON_CLAUSE)==0&&
37083748
!targetIsInSortList(tle,InvalidOid,subquery->distinctClause))
37093749
{
37103750
/* non-DISTINCT column, so mark it unsafe */
3711-
safetyInfo->unsafeColumns[tle->resno]= true;
3751+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_NOTIN_DISTINCTON_CLAUSE;
37123752
continue;
37133753
}
37143754

37153755
/* If subquery uses window functions, check point 4 */
37163756
if (subquery->hasWindowFuncs&&
3757+
(safetyInfo->unsafeFlags[tle->resno]&
3758+
UNSAFE_NOTIN_DISTINCTON_CLAUSE)==0&&
37173759
!targetIsInAllPartitionLists(tle,subquery))
37183760
{
37193761
/* not present in all PARTITION BY clauses, so mark it unsafe */
3720-
safetyInfo->unsafeColumns[tle->resno]= true;
3762+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_NOTIN_PARTITIONBY_CLAUSE;
37213763
continue;
37223764
}
37233765
}
@@ -3729,16 +3771,16 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
37293771
* subquery columns that suffer no type coercions in the set operation.
37303772
* Otherwise there are possible semantic gotchas. So, we check the
37313773
* component queries to see if any of them have output types different from
3732-
* the top-level setop outputs.unsafeColumns[k] issettrue if column k
3733-
* has different type in any component.
3774+
* the top-level setop outputs.Wesetthe UNSAFE_TYPE_MISMATCH bit in
3775+
*unsafeFlags[k] if column khas different type in any component.
37343776
*
37353777
* We don't have to care about typmods here: the only allowed difference
37363778
* between set-op input and output typmods is input is a specific typmod
37373779
* and output is -1, and that does not require a coercion.
37383780
*
37393781
* tlist is a subquery tlist.
37403782
* colTypes is an OID list of the top-level setop's output column types.
3741-
* safetyInfo->unsafeColumns[] is theresult array.
3783+
* safetyInfo is thepushdown_safety_info to set unsafeFlags[] for.
37423784
*/
37433785
staticvoid
37443786
compare_tlist_datatypes(List*tlist,List*colTypes,
@@ -3756,7 +3798,7 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
37563798
if (colType==NULL)
37573799
elog(ERROR,"wrong number of tlist entries");
37583800
if (exprType((Node*)tle->expr)!=lfirst_oid(colType))
3759-
safetyInfo->unsafeColumns[tle->resno]= true;
3801+
safetyInfo->unsafeFlags[tle->resno]|=UNSAFE_TYPE_MISMATCH;
37603802
colType=lnext(colTypes,colType);
37613803
}
37623804
if (colType!=NULL)
@@ -3816,28 +3858,28 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query)
38163858
* 5. rinfo's clause must not refer to any subquery output columns that were
38173859
* found to be unsafe to reference by subquery_is_pushdown_safe().
38183860
*/
3819-
staticbool
3861+
staticpushdown_safe_type
38203862
qual_is_pushdown_safe(Query*subquery,Indexrti,RestrictInfo*rinfo,
38213863
pushdown_safety_info*safetyInfo)
38223864
{
3823-
boolsafe=true;
3865+
pushdown_safe_typesafe=PUSHDOWN_SAFE;
38243866
Node*qual= (Node*)rinfo->clause;
38253867
List*vars;
38263868
ListCell*vl;
38273869

38283870
/* Refuse subselects (point 1) */
38293871
if (contain_subplans(qual))
3830-
returnfalse;
3872+
returnPUSHDOWN_UNSAFE;
38313873

38323874
/* Refuse volatile quals if we found they'd be unsafe (point 2) */
38333875
if (safetyInfo->unsafeVolatile&&
38343876
contain_volatile_functions((Node*)rinfo))
3835-
returnfalse;
3877+
returnPUSHDOWN_UNSAFE;
38363878

38373879
/* Refuse leaky quals if told to (point 3) */
38383880
if (safetyInfo->unsafeLeaky&&
38393881
contain_leaked_vars(qual))
3840-
returnfalse;
3882+
returnPUSHDOWN_UNSAFE;
38413883

38423884
/*
38433885
* Examine all Vars used in clause. Since it's a restriction clause, all
@@ -3864,7 +3906,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
38643906
*/
38653907
if (!IsA(var,Var))
38663908
{
3867-
safe=false;
3909+
safe=PUSHDOWN_UNSAFE;
38683910
break;
38693911
}
38703912

@@ -3876,7 +3918,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
38763918
*/
38773919
if (var->varno!=rti)
38783920
{
3879-
safe=false;
3921+
safe=PUSHDOWN_UNSAFE;
38803922
break;
38813923
}
38823924

@@ -3886,15 +3928,26 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
38863928
/* Check point 4 */
38873929
if (var->varattno==0)
38883930
{
3889-
safe=false;
3931+
safe=PUSHDOWN_UNSAFE;
38903932
break;
38913933
}
38923934

38933935
/* Check point 5 */
3894-
if (safetyInfo->unsafeColumns[var->varattno])
3936+
if (safetyInfo->unsafeFlags[var->varattno]!=0)
38953937
{
3896-
safe= false;
3897-
break;
3938+
if (safetyInfo->unsafeFlags[var->varattno]&
3939+
(UNSAFE_HAS_VOLATILE_FUNC |UNSAFE_HAS_SET_FUNC |
3940+
UNSAFE_NOTIN_DISTINCTON_CLAUSE |UNSAFE_TYPE_MISMATCH))
3941+
{
3942+
safe=PUSHDOWN_UNSAFE;
3943+
break;
3944+
}
3945+
else
3946+
{
3947+
/* UNSAFE_NOTIN_PARTITIONBY_CLAUSE is ok for run conditions */
3948+
safe=PUSHDOWN_WINDOWCLAUSE_RUNCOND;
3949+
/* don't break, we might find another Var that's unsafe */
3950+
}
38983951
}
38993952
}
39003953

0 commit comments

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