8000 Adjust subquery qual pushdown rules to be more forgiving: if a qual · postgrespro/postgres@05f916e · GitHub
[go: up one dir, main page]
Skip to content

Commit 05f916e

Browse files
tglsfdctglsfdc
committed
Adjust subquery qual pushdown rules to be more forgiving: if a qual
refers to a non-DISTINCT output column of a DISTINCT ON subquery, or if it refers to a function-returning-set, we cannot push it down. But the old implementation refused to push down *any* quals if the subquery had any such 'dangerous' outputs. Now we just look at the output columns actually referenced by each qual expression. More code than before, but probably no slower since we don't make unnecessary checks.
1 parent e43094b commit 05f916e

File tree

4 files changed

+110
-62
lines changed

4 files changed

+110
-62
lines changed

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

Lines changed: 100 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.99 2003/03/10 03:53:49 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.100 2003/03/22 01:49:38 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -26,7 +26,9 @@
2626
#include "optimizer/plancat.h"
2727
#include "optimizer/planner.h"
2828
#include "optimizer/prep.h"
29+
#include "optimizer/var.h"
2930
#include "parser/parsetree.h"
31+
#include "parser/parse_clause.h"
3032
#include "rewrite/rewriteManip.h"
3133

3234

@@ -49,6 +51,7 @@ static RelOptInfo *make_one_rel_by_joins(Query *root, int levels_needed,
4951
List *initial_rels);
5052
static bool subquery_is_pushdown_safe(Query *subquery, Query *topquery);
5153
static bool recurse_pushdown_safe(Node *setOp, Query *topquery);
54+
static bool qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual);
5255
static void subquery_push_qual(Query *subquery, Index rti, Node *qual);
5356
static void recurse_push_qual(Node *setOp, Query *topquery,
5457
Index rti, Node *qual);
@@ -305,16 +308,14 @@ set_subquery_pathlist(Query *root, RelOptInfo *rel,
305308
*
306309
* There are several cases where we cannot push down clauses.
307310
* Restrictions involving the subquery are checked by
308-
* subquery_is_pushdown_safe(). Also, we do not push down clauses
309-
* that contain subselects, mainly because I'm not sure it will work
310-
* correctly (the subplan hasn't yet transformed sublinks to
311-
* subselects).
311+
* subquery_is_pushdown_safe(). Restrictions on individual clauses are
312+
* checked by qual_is_pushdown_safe().
312313
*
313314
* Non-pushed-down clauses will get evaluated as qpquals of the
314315
* SubqueryScan node.
315316
*
316317
* XXX Are there any cases where we want to make a policy decision not to
317-
* push down, because it'd result in a worse plan?
318+
* push down a pushable qual, because it'd result in a worse plan?
318319
*/
319320
if (rel->baserestrictinfo != NIL &&
320321
subquery_is_pushdown_safe(subquery, subquery))
@@ -328,15 +329,15 @@ set_subquery_pathlist(Query *root, RelOptInfo *rel,
328329
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lst);
329330
Node *clause = (Node *) rinfo->clause;
330331

331-
if (contain_subplans(clause))
332+
if (qual_is_pushdown_safe(subquery, rti, clause))
332333
{
333-
/* Keep it in the upper query */
334-
upperrestrictlist = lappend(upperrestrictlist, rinfo);
334+
/* Push it down */
335+
subquery_push_qual(subquery, rti, clause);
335336
}
336337
else
337338
{
338-
/* Push it down */
339-
subquery_push_qual(subquery, rti, clause);
339+
/* Keep it in the upper query */
340+
upperrestrictlist = lappend(upperrestrictlist, rinfo);
340341
}
341342
}
342343
rel->baserestrictinfo = upperrestrictlist;
@@ -527,21 +528,10 @@ make_one_rel_by_joins(Query *root, int levels_needed, List *initial_rels)
527528
*
528529
* Conditions checked here:
529530
*
530-
* 1. If the subquery has a LIMIT clause or a DISTINCT ON clause, we must
531-
* not push down any quals, since that could change the set of rows
532-
* returned. (Actually, we could push down quals into a DISTINCT ON
533-
* subquery if they refer only to DISTINCT-ed output columns, but
534-
* checking that seems more work than it's worth. In any case, a
535-
* plain DISTINCT is safe to push down past.)
536-
*
537-
* 2. If the subquery has any functions returning sets in its target list,
538-
* we do not push down any quals, since the quals
539-
* might refer to those tlist items, which would mean we'd introduce
540-
* functions-returning-sets into the subquery's WHERE/HAVING quals.
541-
* (It'd be sufficient to not push down quals that refer to those
542-
* particular tlist items, but that's much clumsier to check.)
531+
* 1. If the subquery has a LIMIT clause, we must not push down any quals,
532+
* since that could change the set of rows returned.
543533
*
544-
* 3. If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot push
534+
* 2. If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot push
545535
* quals into it, because that would change the results. For subqueries
546536
* using UNION/UNION ALL/INTERSECT/INTERSECT ALL, we can push the quals
547537
* into each component query, so long as all the component queries share
@@ -554,11 +544,8 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery)
554544
{
555545
SetOperationStmt *topop;
556546

557-
/* Check points 1 and 2 */
558-
if (subquery->limitOffset != NULL ||
559-
subquery->limitCount != NULL ||
560-
has_distinct_on_clause(subquery) ||
561-
expression_returns_set((Node *) subquery->targetList))
547+
/* Check point 1 */
548+
if (subquery->limitOffset != NULL || subquery->limitCount != NULL)
562549
return false;
563550

564551
/* Are we at top level, or looking at a setop component? */
@@ -622,6 +609,89 @@ recurse_pushdown_safe(Node *setOp, Query *topquery)
622609
return true;
623610
}
624611

612+
/*
613+
* qual_is_pushdown_safe - is a particular qual safe to push down?
614+
*
615+
* qual is a restriction clause applying to the given subquery (whose RTE
616+
* has index rti in the parent query).
617+
*
618+
* Conditions checked here:
619+
*
620+
* 1. The qual must not contain any subselects (mainly because I'm not sure
621+
* it will work correctly: sublinks will already have been transformed into
622+
* subplans in the qual, but not in the subquery).
623+
*
624+
* 2. If the subquery uses DISTINCT ON, we must not push down any quals that
625+
* refer to non-DISTINCT output columns, because that could change the set
626+
* of rows returned. This condition is vacuous for DISTINCT, because then
627+
* there are no non-DISTINCT output columns, but unfortunately it's fairly
628+
* expensive to tell the difference between DISTINCT and DISTINCT ON in the
629+
* parsetree representation. It's cheaper to just make sure all the Vars
630+
* in the qual refer to DISTINCT columns.
631+
*
632+
* 3. We must not push down any quals that refer to subselect outputs that
633+
* return sets, else we'd introduce functions-returning-sets into the
634+
* subquery's WHERE/HAVING quals.
635+
*/
636+
static bool
637+
qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual)
638+
{
639+
bool safe = true;
640+
List *vars;
641+
List *l;
642+
Bitmapset *tested = NULL;
643+
644+
/* Refuse subselects (point 1) */
645+
if (contain_subplans(qual))
646+
return false;
647+
648+
/*
649+
* Examine all Vars used in clause; since it's a restriction clause,
650+
* all such Vars must refer to subselect output columns.
651+
*/
652+
vars = pull_var_clause(qual, false);
653+
foreach(l, vars)
654+
{
655+
Var *var = (Var *) lfirst(l);
656+
TargetEntry *tle;
657+
658+
Assert(var->varno == rti);
659+
/*
660+
* We use a bitmapset to avoid testing the same attno more than
661+
* once. (NB: this only works because subquery outputs can't
662+
* have negative attnos.)
663+
*/
664+
if (bms_is_member(var->varattno, tested))
665+
continue;
666+
tested = bms_add_member(tested, var->varattno);
667+
668+
tle = (TargetEntry *) nth(var->varattno-1, subquery->targetList);
669+
Assert(tle->resdom->resno == var->varattno);
670+
Assert(!tle->resdom->resjunk);
671+
672+
/* If subquery uses DISTINCT or DISTINCT ON, check point 2 */
673+
if (subquery->distinctClause != NIL &&
674+
!targetIsInSortList(tle, subquery->distinctClause))
675+
{
676+
/* non-DISTINCT column, so fail */
677+
safe = false;
678+
break;
679+
}
680+
681+
/* Refuse functions returning sets (point 3) */
682+
if (expression_returns_set((Node *) tle->expr))
683+
{
684+
safe = false;
685+
break;
686+
}
687+
}
688+
689+
freeList(vars);
690+
bms_free(tested);
691+
692+
return safe;
693+
}
694+
625695
/*
626696
* subquery_push_qual - push down a qual that we have determined is safe
627697
*/

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

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.132 2003/03/14 00:55:17 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.133 2003/03/22 01:49:38 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -29,6 +29,7 @@
2929
#include "optimizer/clauses.h"
3030
#include "optimizer/var.h"
3131
#include "parser/analyze.h"
32+
#include "parser/parse_clause.h"
3233
#include "tcop/tcopprot.h"
3334
#include "utils/acl.h"
3435
#include "utils/builtins.h"
@@ -813,28 +814,6 @@ pull_constant_clauses(List *quals, List **constantQual)
813814
* think of a better place for it...
814815
*****************************************************************************/
815816

816-
/*
817-
* Test whether a sort/group reference value appears in the given list of
818-
* SortClause (or GroupClause) nodes.
819-
*
820-
* Because GroupClause is typedef'd as SortClause, either kind of
821-
* node list can be passed without casting.
822-
*/
823-
static bool
824-
sortgroupref_is_present(Index sortgroupref, List *clauselist)
825-
{
826-
List *clause;
827-
828-
foreach(clause, clauselist)
829-
{
830-
SortClause *scl = (SortClause *) lfirst(clause);
831-
832-
if (scl->tleSortGroupRef == sortgroupref)
833-
return true;
834-
}
835-
return false;
836-
}
837-
838817
/*
839818
* Test whether a query uses DISTINCT ON, ie, has a distinct-list that is
840819
* not the same as the set of output columns.
@@ -864,15 +843,14 @@ has_distinct_on_clause(Query *query)
864843
foreach(targetList, query->targetList)
865844
{
866845
TargetEntry *tle = (TargetEntry *) lfirst(targetList);
867-
Index ressortgroupref = tle->resdom->ressortgroupref;
868846

869-
if (ressortgroupref == 0)
847+
if (tle->resdom->ressortgroupref == 0)
870848
{
871849
if (tle->resdom->resjunk)
872850
continue; /* we can ignore unsorted junk cols */
873851
return true; /* definitely not in DISTINCT list */
874852
}
875-
if (sortgroupref_is_present(ressortgroupref, query->distinctClause))
853+
if (targetIsInSortList(tle, query->distinctClause))
876854
{
877855
if (tle->resdom->resjunk)
878856
return true; /* junk TLE in DISTINCT means DISTINCT ON */
@@ -883,7 +861,7 @@ has_distinct_on_clause(Query *query)
883861
/* This TLE is not in DISTINCT list */
884862
if (!tle->resdom->resjunk)
885863
return true; /* non-junk, non-DISTINCT, so DISTINCT ON */
886-
if (sortgroupref_is_present(ressortgroupref, query->sortClause))
864+
if (targetIsInSortList(tle, query->sortClause))
887865
return true; /* sorted, non-distinct junk */
888866
/* unsorted junk is okay, keep looking */
889867
}

‎src/backend/parser/parse_clause.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.111 2003/03/10 03:53:51 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.112 2003/03/22 01:49:38 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -60,7 +60,6 @@ static TargetEntry *findTargetlistEntry(ParseState *pstate, Node *node,
6060
List *tlist, int clause);
6161
static List *addTargetToSortList(TargetEntry *tle, List *sortlist,
6262
List *targetlist, List *opname);
63-
static bool targetIsInSortList(TargetEntry *tle, List *sortList);
6463

6564

6665
/*
@@ -1386,7 +1385,7 @@ assignSortGroupRef(TargetEntry *tle, List *tlist)
13861385
* reason we need this routine (and not just a quick test for nonzeroness
13871386
* of ressortgroupref) is that a TLE might be in only one of the lists.
13881387
*/
1389-
static bool
1388+
bool
13901389
targetIsInSortList(TargetEntry *tle, List *sortList)
13911390
{
13921391
Index ref = tle->resdom->ressortgroupref;

‎src/include/parser/parse_clause.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
/*-------------------------------------------------------------------------
22
*
33
* parse_clause.h
4-
*
4+
* handle clauses in parser
55
*
66
*
77
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: parse_clause.h,v 1.29 2002/06/20 20:29:51 momjian Exp $
10+
* $Id: parse_clause.h,v 1.30 2003/03/22 01:49:38 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -30,5 +30,6 @@ extern List *transformDistinctClause(ParseState *pstate, List *distinctlist,
3030

3131
extern List *addAllTargetsToSortList(List *sortlist, List *targetlist);
3232
extern Index assignSortGroupRef(TargetEntry *tle, List *tlist);
33+
extern bool targetIsInSortList(TargetEntry *tle, List *sortList);
3334

3435
#endif /* PARSE_CLAUSE_H */

0 commit comments

Comments
 (0)
0