Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 421467c

Browse files
committed
Fix optimizer to not try to push WHERE clauses down into a sub-SELECT that
has a DISTINCT ON clause, per bug report from Anthony Wood. While at it, improve the DISTINCT-ON-clause recognizer routine to not be fooled by out- of-order DISTINCT lists.
1 parent 6fbf442 commit 421467c

File tree

4 files changed

+70
-40
lines changed

4 files changed

+70
-40
lines changed

src/backend/optimizer/path/allpaths.c

Lines changed: 9 additions & 4 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.77 2001/07/16 17:57:02 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.78 2001/07/31 17:56:30 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -296,8 +296,12 @@ set_subquery_pathlist(Query *root, RelOptInfo *rel,
296296
* the quals down into the component queries of the setop, but getting it
297297
* right seems nontrivial. Work on this later.)
298298
*
299-
* 2. If the subquery has a LIMIT clause we must not push down any quals,
300-
* since that could change the set of rows returned.
299+
* 2. If the subquery has a LIMIT clause or a DISTINCT ON clause, we must
300+
* not push down any quals, since that could change the set of rows
301+
* returned. (Actually, we could push down quals into a DISTINCT ON
302+
* subquery if they refer only to DISTINCT-ed output columns, but checking
303+
* that seems more work than it's worth. In any case, a plain DISTINCT is
304+
* safe to push down past.)
301305
*
302306
* 3. We do not push down clauses that contain subselects, mainly because
303307
* I'm not sure it will work correctly (the subplan hasn't yet transformed
@@ -311,7 +315,8 @@ set_subquery_pathlist(Query *root, RelOptInfo *rel,
311315
*/
312316
if (subquery->setOperations == NULL &&
313317
subquery->limitOffset == NULL &&
314-
subquery->limitCount == NULL)
318+
subquery->limitCount == NULL &&
319+
!has_distinct_on_clause(subquery))
315320
{
316321
/* OK to consider pushing down individual quals */
317322
List *upperrestrictlist = NIL;

src/backend/optimizer/util/clauses.c

Lines changed: 54 additions & 2 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.86 2001/06/19 22:39:11 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.87 2001/07/31 17:56:31 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -728,14 +728,66 @@ pull_constant_clauses(List *quals, List **constantQual)
728728
}
729729

730730

731+
/*****************************************************************************
732+
* Tests on clauses of queries
733+
*
734+
* Possibly this code should go someplace else, since this isn't quite the
735+
* same meaning of "clause" as is used elsewhere in this module. But I can't
736+
* think of a better place for it...
737+
*****************************************************************************/
738+
739+
/*
740+
* Test whether a query uses DISTINCT ON, ie, has a distinct-list that is
741+
* just a subset of the output columns.
742+
*/
743+
bool
744+
has_distinct_on_clause(Query *query)
745+
{
746+
List *targetList;
747+
748+
/* Is there a DISTINCT clause at all? */
749+
if (query->distinctClause == NIL)
750+
return false;
751+
/*
752+
* If the DISTINCT list contains all the nonjunk targetlist items,
753+
* then it's a simple DISTINCT, else it's DISTINCT ON. We do not
754+
* require the lists to be in the same order (since the parser may
755+
* have adjusted the DISTINCT clause ordering to agree with ORDER BY).
756+
*/
757+
foreach(targetList, query->targetList)
758+
{
759+
TargetEntry *tle = (TargetEntry *) lfirst(targetList);
760+
Index ressortgroupref;
761+
List *distinctClause;
762+
763+
if (tle->resdom->resjunk)
764+
continue;
765+
ressortgroupref = tle->resdom->ressortgroupref;
766+
if (ressortgroupref == 0)
767+
return true; /* definitely not in DISTINCT list */
768+
foreach(distinctClause, query->distinctClause)
769+
{
770+
SortClause *scl = (SortClause *) lfirst(distinctClause);
771+
772+
if (scl->tleSortGroupRef == ressortgroupref)
773+
break; /* found TLE in DISTINCT */
774+
}
775+
if (distinctClause == NIL)
776+
return true; /* this TLE is not in DISTINCT list */
777+
}
778+
/* It's a simple DISTINCT */
779+
return false;
780+
}
781+
782+
731783
/*****************************************************************************
732784
* *
733785
* General clause-manipulating routines *
734786
* *
735787
*****************************************************************************/
736788

737789
/*
738-
* clause_relids_vars
790+
* clause_get_relids_vars
739791
* Retrieves distinct relids and vars appearing within a clause.
740792
*
741793
* '*relids' is set to an integer list of all distinct "varno"s appearing

src/backend/utils/adt/ruleutils.c

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* back to source text
44
*
55
* IDENTIFICATION
6-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.80 2001/07/16 05:06:59 tgl Exp $
6+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.81 2001/07/31 17:56:31 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -119,7 +119,6 @@ static void get_utility_query_def(Query *query, deparse_context *context);
119119
static void get_basic_select_query(Query *query, deparse_context *context);
120120
static void get_setop_query(Node *setOp, Query *query,
121121
deparse_context *context, bool toplevel);
122-
static bool simple_distinct(List *distinctClause, List *targetList);
123122
static void get_rule_sortgroupclause(SortClause *srt, List *tlist,
124123
bool force_colno,
125124
deparse_context *context);
@@ -1064,9 +1063,7 @@ get_basic_select_query(Query *query, deparse_context *context)
10641063
/* Add the DISTINCT clause if given */
10651064
if (query->distinctClause != NIL)
10661065
{
1067-
if (simple_distinct(query->distinctClause, query->targetList))
1068-
appendStringInfo(buf, " DISTINCT");
1069-
else
1066+
if (has_distinct_on_clause(query))
10701067
{
10711068
appendStringInfo(buf, " DISTINCT ON (");
10721069
sep = "";
@@ -1081,6 +1078,8 @@ get_basic_select_query(Query *query, deparse_context *context)
10811078
}
10821079
appendStringInfo(buf, ")");
10831080
}
1081+
else
1082+
appendStringInfo(buf, " DISTINCT");
10841083
}
10851084

10861085
/* Then we tell what to select (the targetlist) */
@@ -1207,34 +1206,6 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
12071206
}
12081207
}
12091208

1210-
/*
1211-
* Detect whether a DISTINCT list can be represented as just DISTINCT
1212-
* or needs DISTINCT ON. It's simple if it contains exactly the nonjunk
1213-
* targetlist items.
1214-
*/
1215-
static bool
1216-
simple_distinct(List *distinctClause, List *targetList)
1217-
{
1218-
while (targetList)
1219-
{
1220-
TargetEntry *tle = (TargetEntry *) lfirst(targetList);
1221-
1222-
if (!tle->resdom->resjunk)
1223-
{
1224-
if (distinctClause == NIL)
1225-
return false;
1226-
if (((SortClause *) lfirst(distinctClause))->tleSortGroupRef !=
1227-
tle->resdom->ressortgroupref)
1228-
return false;
1229-
distinctClause = lnext(distinctClause);
1230-
}
1231-
targetList = lnext(targetList);
1232-
}
1233-
if (distinctClause != NIL)
1234-
return false;
1235-
return true;
1236-
}
1237-
12381209
/*
12391210
* Display a sort/group clause.
12401211
*/

src/include/optimizer/clauses.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: clauses.h,v 1.44 2001/05/20 20:28:20 tgl Exp $
10+
* $Id: clauses.h,v 1.45 2001/07/31 17:56:31 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -51,6 +51,8 @@ extern bool contain_noncachable_functions(Node *clause);
5151
extern bool is_pseudo_constant_clause(Node *clause);
5252
extern List *pull_constant_clauses(List *quals, List **constantQual);
5353

54+
extern bool has_distinct_on_clause(Query *query);
55+
5456
extern void clause_get_relids_vars(Node *clause, Relids *relids, List **vars);
5557
extern int NumRelids(Node *clause);
5658
extern void CommuteClause(Expr *clause);

0 commit comments

Comments
 (0)