Skip to content

Commit a1e17cd

Browse files
tglsfdctglsfdc
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. Also, back-patch earlier fix to not push down into sub-SELECT with LIMIT.
1 parent d6b1a40 commit a1e17cd

File tree

4 files changed

+69
-38
lines changed

4 files changed

+69
-38
lines changed

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

Lines changed: 8 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/path/allpaths.c,v 1.72 2001/03/22 03:59:34 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.72.2.1 2001/07/31 18:39:12 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -125,11 +125,17 @@ set_base_rel_pathlists(Query *root)
125125
* Non-pushed-down clauses will get evaluated as qpquals of
126126
* the SubqueryScan node.
127127
*
128+
* We can't push down into subqueries with LIMIT or DISTINCT ON
129+
* clauses, either.
130+
*
128131
* XXX Are there any cases where we want to make a policy
129132
* decision not to push down, because it'd result in a worse
130133
* plan?
131134
*/
132-
if (rte->subquery->setOperations == NULL)
135+
if (rte->subquery->setOperations == NULL &&
136+
rte->subquery->limitOffset == NULL &&
137+
rte->subquery->limitCount == NULL &&
138+
!has_distinct_on_clause(rte->subquery))
133139
{
134140
/* OK to consider pushing down individual quals */
135141
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.84 2001/03/27 17:12:34 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.84.2.1 2001/07/31 18:39:12 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -729,14 +729,66 @@ pull_constant_clauses(List *quals, List **constantQual)
729729
}
730730

731731

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

738790
/*
739-
* clause_relids_vars
791+
* clause_get_relids_vars
740792
* Retrieves distinct relids and vars appearing within a clause.
741793
*
742794
* '*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.77 2001/04/18 17:04:24 tgl Exp $
6+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.77.2.1 2001/07/31 18:39:12 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);
@@ -1006,9 +1005,7 @@ get_basic_select_query(Query *query, deparse_context *context)
10061005
/* Add the DISTINCT clause if given */
10071006
if (query->distinctClause != NIL)
10081007
{
1009-
if (simple_distinct(query->distinctClause, query->targetList))
1010-
appendStringInfo(buf, " DISTINCT");
1011-
else
1008+
if (has_distinct_on_clause(query))
10121009
{
10131010
appendStringInfo(buf, " DISTINCT ON (");
10141011
sep = "";
@@ -1023,6 +1020,8 @@ get_basic_select_query(Query *query, deparse_context *context)
10231020
}
10241021
appendStringInfo(buf, ")");
10251022
}
1023+
else
1024+
appendStringInfo(buf, " DISTINCT");
10261025
}
10271026

10281027
/* Then we tell what to select (the targetlist) */
@@ -1149,34 +1148,6 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
11491148
}
11501149
}
11511150

1152-
/*
1153-
* Detect whether a DISTINCT list can be represented as just DISTINCT
1154-
* or needs DISTINCT ON. It's simple if it contains exactly the nonjunk
1155-
* targetlist items.
1156-
*/
1157-
static bool
1158-
simple_distinct(List *distinctClause, List *targetList)
1159-
{
1160-
while (targetList)
1161-
{
1162-
TargetEntry *tle = (TargetEntry *) lfirst(targetList);
1163-
1164-
if (!tle->resdom->resjunk)
1165-
{
1166-
if (distinctClause == NIL)
1167-
return false;
1168-
if (((SortClause *) lfirst(distinctClause))->tleSortGroupRef !=
1169-
tle->resdom->ressortgroupref)
1170-
return false;
1171-
distinctClause = lnext(distinctClause);
1172-
}
1173-
targetList = lnext(targetList);
1174-
}
1175-
if (distinctClause != NIL)
1176-
return false;
1177-
return true;
1178-
}
1179-
11801151
/*
11811152
* Display a sort/group clause.
11821153
*/

‎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.43 2001/03/22 04:00:53 momjian Exp $
10+
* $Id: clauses.h,v 1.43.2.1 2001/07/31 18:39:13 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -59,6 +59,8 @@ extern bool contain_noncachable_functions(Node *clause);
5959
extern bool is_pseudo_constant_clause(Node *clause);
6060
extern List *pull_constant_clauses(List *quals, List **constantQual);
6161

62+
extern bool has_distinct_on_clause(Query *query);
63+
6264
extern void clause_get_relids_vars(Node *clause, Relids *relids, List **vars);
6365
extern int NumRelids(Node *clause);
6466
extern void get_relattval(Node *clause, int targetrelid,

0 commit comments

Comments
 (0)