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

Commitf753c9d

Browse files
committed
Prevent pushing down WHERE clauses into unsafe UNION/INTERSECT nests.
The planner is aware that it mustn't push down upper-level quals intosubqueries if the quals reference subquery output columns that containset-returning functions or volatile functions, or are non-DISTINCT outputsof a DISTINCT ON subquery. However, it missed making this check whenthere were one or more levels of UNION or INTERSECT above the dangerousexpression. This could lead to "set-valued function called in context thatcannot accept a set" errors, as seen in bug #8213 from Eric Soroos, or tosilently wrong answers in the other cases.To fix, refactor the checks so that we make the column-is-unsafe checksduring subquery_is_pushdown_safe(), which already has to recursivelyinspect all arms of a set-operation tree. This makesqual_is_pushdown_safe() considerably simpler, at the cost that we willspend some cycles checking output columns that possibly aren't referencedin any upper qual. But the cases where this code gets executed at allare already nontrivial queries, so it's unlikely anybody will notice anyslowdown of planning.This has been broken since commit05f916e,which makes the bug over ten years old. A bit surprising nobody noticed itbefore now.
1 parent940a85e commitf753c9d

File tree

3 files changed

+257
-91
lines changed

3 files changed

+257
-91
lines changed

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

Lines changed: 125 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,14 @@ static void set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel,
6969
RangeTblEntry*rte);
7070
staticRelOptInfo*make_rel_from_joinlist(PlannerInfo*root,List*joinlist);
7171
staticboolsubquery_is_pushdown_safe(Query*subquery,Query*topquery,
72-
bool*differentTypes);
72+
bool*unsafeColumns);
7373
staticboolrecurse_pushdown_safe(Node*setOp,Query*topquery,
74-
bool*differentTypes);
74+
bool*unsafeColumns);
75+
staticvoidcheck_output_expressions(Query*subquery,bool*unsafeColumns);
7576
staticvoidcompare_tlist_datatypes(List*tlist,List*colTypes,
76-
bool*differentTypes);
77+
bool*unsafeColumns);
7778
staticboolqual_is_pushdown_safe(Query*subquery,Indexrti,Node*qual,
78-
bool*differentTypes);
79+
bool*unsafeColumns);
7980
staticvoidsubquery_push_qual(Query*subquery,
8081
RangeTblEntry*rte,Indexrti,Node*qual);
8182
staticvoidrecurse_push_qual(Node*setOp,Query*topquery,
@@ -705,7 +706,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
705706
{
706707
Query*parse=root->parse;
707708
Query*subquery=rte->subquery;
708-
bool*differentTypes;
709+
bool*unsafeColumns;
709710
doubletuple_fraction;
710711
PlannerInfo*subroot;
711712
List*pathkeys;
@@ -717,8 +718,12 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
717718
*/
718719
subquery=copyObject(subquery);
719720

720-
/* We need a workspace for keeping track of set-op type coercions */
721-
differentTypes= (bool*)
721+
/*
722+
* We need a workspace for keeping track of unsafe-to-reference columns.
723+
* unsafeColumns[i] is set TRUE if we've found that output column i of the
724+
* subquery is unsafe to use in a pushed-down qual.
725+
*/
726+
unsafeColumns= (bool*)
722727
palloc0((list_length(subquery->targetList)+1)*sizeof(bool));
723728

724729
/*
@@ -742,7 +747,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
742747
* push down a pushable qual, because it'd result in a worse plan?
743748
*/
744749
if (rel->baserestrictinfo!=NIL&&
745-
subquery_is_pushdown_safe(subquery,subquery,differentTypes))
750+
subquery_is_pushdown_safe(subquery,subquery,unsafeColumns))
746751
{
747752
/* OK to consider pushing down individual quals */
748753
List*upperrestrictlist=NIL;
@@ -754,7 +759,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
754759
Node*clause= (Node*)rinfo->clause;
755760

756761
if (!rinfo->pseudoconstant&&
757-
qual_is_pushdown_safe(subquery,rti,clause,differentTypes))
762+
qual_is_pushdown_safe(subquery,rti,clause,unsafeColumns))
758763
{
759764
/* Push it down */
760765
subquery_push_qual(subquery,rte,rti,clause);
@@ -768,7 +773,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
768773
rel->baserestrictinfo=upperrestrictlist;
769774
}
770775

771-
pfree(differentTypes);
776+
pfree(unsafeColumns);
772777

773778
/*
774779
* We can safely pass the outer tuple_fraction down to the subquery if the
@@ -1169,17 +1174,19 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
11691174
* 3. If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot push
11701175
* quals into it, because that could change the results.
11711176
*
1172-
* 4. For subqueries using UNION/UNION ALL/INTERSECT/INTERSECT ALL, we can
1173-
* push quals into each component query, but the quals can only reference
1174-
* subquery columns that suffer no type coercions in the set operation.
1175-
* Otherwise there are possible semantic gotchas. So, we check the
1176-
* component queries to see if any of them have different output types;
1177-
* differentTypes[k] is set true if column k has different type in any
1178-
* component.
1177+
* In addition, we make several checks on the subquery's output columns
1178+
* to see if it is safe to reference them in pushed-down quals. If output
1179+
* column k is found to be unsafe to reference, we set unsafeColumns[k] to
1180+
* TRUE, but we don't reject the subquery overall since column k might
1181+
* not be referenced by some/all quals. The unsafeColumns[] array will be
1182+
* consulted later by qual_is_pushdown_safe().It's better to do it this
1183+
* way than to make the checks directly in qual_is_pushdown_safe(), because
1184+
* when the subquery involves set operations we have to check the output
1185+
* expressions in each arm of the set op.
11791186
*/
11801187
staticbool
11811188
subquery_is_pushdown_safe(Query*subquery,Query*topquery,
1182-
bool*differentTypes)
1189+
bool*unsafeColumns)
11831190
{
11841191
SetOperationStmt*topop;
11851192

@@ -1191,13 +1198,22 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
11911198
if (subquery->hasWindowFuncs)
11921199
return false;
11931200

1201+
/*
1202+
* If we're at a leaf query, check for unsafe expressions in its target
1203+
* list, and mark any unsafe ones in unsafeColumns[]. (Non-leaf nodes in
1204+
* setop trees have only simple Vars in their tlists, so no need to check
1205+
* them.)
1206+
*/
1207+
if (subquery->setOperations==NULL)
1208+
check_output_expressions(subquery,unsafeColumns);
1209+
11941210
/* Are we at top level, or looking at a setop component? */
11951211
if (subquery==topquery)
11961212
{
11971213
/* Top level, so check any component queries */
11981214
if (subquery->setOperations!=NULL)
11991215
if (!recurse_pushdown_safe(subquery->setOperations,topquery,
1200-
differentTypes))
1216+
unsafeColumns))
12011217
return false;
12021218
}
12031219
else
@@ -1210,7 +1226,7 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
12101226
Assert(topop&&IsA(topop,SetOperationStmt));
12111227
compare_tlist_datatypes(subquery->targetList,
12121228
topop->colTypes,
1213-
differentTypes);
1229+
unsafeColumns);
12141230
}
12151231
return true;
12161232
}
@@ -1220,7 +1236,7 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
12201236
*/
12211237
staticbool
12221238
recurse_pushdown_safe(Node*setOp,Query*topquery,
1223-
bool*differentTypes)
1239+
bool*unsafeColumns)
12241240
{
12251241
if (IsA(setOp,RangeTblRef))
12261242
{
@@ -1229,19 +1245,19 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
12291245
Query*subquery=rte->subquery;
12301246

12311247
Assert(subquery!=NULL);
1232-
returnsubquery_is_pushdown_safe(subquery,topquery,differentTypes);
1248+
returnsubquery_is_pushdown_safe(subquery,topquery,unsafeColumns);
12331249
}
12341250
elseif (IsA(setOp,SetOperationStmt))
12351251
{
12361252
SetOperationStmt*op= (SetOperationStmt*)setOp;
12371253

1238-
/* EXCEPT is no good */
1254+
/* EXCEPT is no good(point 3 for subquery_is_pushdown_safe)*/
12391255
if (op->op==SETOP_EXCEPT)
12401256
return false;
12411257
/* Else recurse */
1242-
if (!recurse_pushdown_safe(op->larg,topquery,differentTypes))
1258+
if (!recurse_pushdown_safe(op->larg,topquery,unsafeColumns))
12431259
return false;
1244-
if (!recurse_pushdown_safe(op->rarg,topquery,differentTypes))
1260+
if (!recurse_pushdown_safe(op->rarg,topquery,unsafeColumns))
12451261
return false;
12461262
}
12471263
else
@@ -1253,17 +1269,92 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
12531269
}
12541270

12551271
/*
1256-
* Compare tlist's datatypes against the list of set-operation result types.
1257-
* For any items that are different, mark the appropriate element of
1258-
* differentTypes[] to show that this column will have type conversions.
1272+
* check_output_expressions - check subquery's output expressions for safety
1273+
*
1274+
* There are several cases in which it's unsafe to push down an upper-level
1275+
* qual if it references a particular output column of a subquery.We check
1276+
* each output column of the subquery and set unsafeColumns[k] to TRUE if
1277+
* that column is unsafe for a pushed-down qual to reference. The conditions
1278+
* checked here are:
1279+
*
1280+
* 1. We must not push down any quals that refer to subselect outputs that
1281+
* return sets, else we'd introduce functions-returning-sets into the
1282+
* subquery's WHERE/HAVING quals.
1283+
*
1284+
* 2. We must not push down any quals that refer to subselect outputs that
1285+
* contain volatile functions, for fear of introducing strange results due
1286+
* to multiple evaluation of a volatile function.
1287+
*
1288+
* 3. If the subquery uses DISTINCT ON, we must not push down any quals that
1289+
* refer to non-DISTINCT output columns, because that could change the set
1290+
* of rows returned. (This condition is vacuous for DISTINCT, because then
1291+
* there are no non-DISTINCT output columns, so we needn't check. But note
1292+
* we are assuming that the qual can't distinguish values that the DISTINCT
1293+
* operator sees as equal.This is a bit shaky but we have no way to test
1294+
* for the case, and it's unlikely enough that we shouldn't refuse the
1295+
* optimization just because it could theoretically happen.)
1296+
*/
1297+
staticvoid
1298+
check_output_expressions(Query*subquery,bool*unsafeColumns)
1299+
{
1300+
ListCell*lc;
1301+
1302+
foreach(lc,subquery->targetList)
1303+
{
1304+
TargetEntry*tle= (TargetEntry*)lfirst(lc);
1305+
1306+
if (tle->resjunk)
1307+
continue;/* ignore resjunk columns */
1308+
1309+
/* We need not check further if output col is already known unsafe */
1310+
if (unsafeColumns[tle->resno])
1311+
continue;
1312+
1313+
/* Functions returning sets are unsafe (point 1) */
1314+
if (expression_returns_set((Node*)tle->expr))
1315+
{
1316+
unsafeColumns[tle->resno]= true;
1317+
continue;
1318+
}
1319+
1320+
/* Volatile functions are unsafe (point 2) */
1321+
if (contain_volatile_functions((Node*)tle->expr))
1322+
{
1323+
unsafeColumns[tle->resno]= true;
1324+
continue;
1325+
}
1326+
1327+
/* If subquery uses DISTINCT ON, check point 3 */
1328+
if (subquery->hasDistinctOn&&
1329+
!targetIsInSortList(tle,InvalidOid,subquery->distinctClause))
1330+
{
1331+
/* non-DISTINCT column, so mark it unsafe */
1332+
unsafeColumns[tle->resno]= true;
1333+
continue;
1334+
}
1335+
}
1336+
}
1337+
1338+
/*
1339+
* For subqueries using UNION/UNION ALL/INTERSECT/INTERSECT ALL, we can
1340+
* push quals into each component query, but the quals can only reference
1341+
* subquery columns that suffer no type coercions in the set operation.
1342+
* Otherwise there are possible semantic gotchas. So, we check the
1343+
* component queries to see if any of them have output types different from
1344+
* the top-level setop outputs. unsafeColumns[k] is set true if column k
1345+
* has different type in any component.
12591346
*
12601347
* We don't have to care about typmods here: the only allowed difference
12611348
* between set-op input and output typmods is input is a specific typmod
12621349
* and output is -1, and that does not require a coercion.
1350+
*
1351+
* tlist is a subquery tlist.
1352+
* colTypes is an OID list of the top-level setop's output column types.
1353+
* unsafeColumns[] is the result array.
12631354
*/
12641355
staticvoid
12651356
compare_tlist_datatypes(List*tlist,List*colTypes,
1266-
bool*differentTypes)
1357+
bool*unsafeColumns)
12671358
{
12681359
ListCell*l;
12691360
ListCell*colType=list_head(colTypes);
@@ -1277,7 +1368,7 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
12771368
if (colType==NULL)
12781369
elog(ERROR,"wrong number of tlist entries");
12791370
if (exprType((Node*)tle->expr)!=lfirst_oid(colType))
1280-
differentTypes[tle->resno]= true;
1371+
unsafeColumns[tle->resno]= true;
12811372
colType=lnext(colType);
12821373
}
12831374
if (colType!=NULL)
@@ -1300,34 +1391,15 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
13001391
* (since there is no easy way to name that within the subquery itself).
13011392
*
13021393
* 3. The qual must not refer to any subquery output columns that were
1303-
* found to have inconsistent types across a set operation tree by
1304-
* subquery_is_pushdown_safe().
1305-
*
1306-
* 4. If the subquery uses DISTINCT ON, we must not push down any quals that
1307-
* refer to non-DISTINCT output columns, because that could change the set
1308-
* of rows returned. (This condition is vacuous for DISTINCT, because then
1309-
* there are no non-DISTINCT output columns, so we needn't check. But note
1310-
* we are assuming that the qual can't distinguish values that the DISTINCT
1311-
* operator sees as equal.This is a bit shaky but we have no way to test
1312-
* for the case, and it's unlikely enough that we shouldn't refuse the
1313-
* optimization just because it could theoretically happen.)
1314-
*
1315-
* 5. We must not push down any quals that refer to subselect outputs that
1316-
* return sets, else we'd introduce functions-returning-sets into the
1317-
* subquery's WHERE/HAVING quals.
1318-
*
1319-
* 6. We must not push down any quals that refer to subselect outputs that
1320-
* contain volatile functions, for fear of introducing strange results due
1321-
* to multiple evaluation of a volatile function.
1394+
* found to be unsafe to reference by subquery_is_pushdown_safe().
13221395
*/
13231396
staticbool
13241397
qual_is_pushdown_safe(Query*subquery,Indexrti,Node*qual,
1325-
bool*differentTypes)
1398+
bool*unsafeColumns)
13261399
{
13271400
boolsafe= true;
13281401
List*vars;
13291402
ListCell*vl;
1330-
Bitmapset*tested=NULL;
13311403

13321404
/* Refuse subselects (point 1) */
13331405
if (contain_subplans(qual))
@@ -1350,7 +1422,6 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
13501422
foreach(vl,vars)
13511423
{
13521424
Var*var= (Var*)lfirst(vl);
1353-
TargetEntry*tle;
13541425

13551426
/*
13561427
* XXX Punt if we find any PlaceHolderVars in the restriction clause.
@@ -1366,6 +1437,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
13661437
}
13671438

13681439
Assert(var->varno==rti);
1440+
Assert(var->varattno >=0);
13691441

13701442
/* Check point 2 */
13711443
if (var->varattno==0)
@@ -1374,53 +1446,15 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
13741446
break;
13751447
}
13761448

1377-
/*
1378-
* We use a bitmapset to avoid testing the same attno more than once.
1379-
* (NB: this only works because subquery outputs can't have negative
1380-
* attnos.)
1381-
*/
1382-
if (bms_is_member(var->varattno,tested))
1383-
continue;
1384-
tested=bms_add_member(tested,var->varattno);
1385-
13861449
/* Check point 3 */
1387-
if (differentTypes[var->varattno])
1388-
{
1389-
safe= false;
1390-
break;
1391-
}
1392-
1393-
/* Must find the tlist element referenced by the Var */
1394-
tle=get_tle_by_resno(subquery->targetList,var->varattno);
1395-
Assert(tle!=NULL);
1396-
Assert(!tle->resjunk);
1397-
1398-
/* If subquery uses DISTINCT ON, check point 4 */
1399-
if (subquery->hasDistinctOn&&
1400-
!targetIsInSortList(tle,InvalidOid,subquery->distinctClause))
1401-
{
1402-
/* non-DISTINCT column, so fail */
1403-
safe= false;
1404-
break;
1405-
}
1406-
1407-
/* Refuse functions returning sets (point 5) */
1408-
if (expression_returns_set((Node*)tle->expr))
1409-
{
1410-
safe= false;
1411-
break;
1412-
}
1413-
1414-
/* Refuse volatile functions (point 6) */
1415-
if (contain_volatile_functions((Node*)tle->expr))
1450+
if (unsafeColumns[var->varattno])
14161451
{
14171452
safe= false;
14181453
break;
14191454
}
14201455
}
14211456

14221457
list_free(vars);
1423-
bms_free(tested);
14241458

14251459
returnsafe;
14261460
}

0 commit comments

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