]> granicus.if.org Git - postgresql/commitdiff
Fix optimizer to not try to push WHERE clauses down into a sub-SELECT that
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 31 Jul 2001 17:56:31 +0000 (17:56 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 31 Jul 2001 17:56:31 +0000 (17:56 +0000)
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.

src/backend/optimizer/path/allpaths.c
src/backend/optimizer/util/clauses.c
src/backend/utils/adt/ruleutils.c
src/include/optimizer/clauses.h

index f378486c83da0e34a402b62dd76497f4d1e88359..8a6e9b9374a6a5b00f2654a08b80e4bd508c80d9 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.77 2001/07/16 17:57:02 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.78 2001/07/31 17:56:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -296,8 +296,12 @@ set_subquery_pathlist(Query *root, RelOptInfo *rel,
         * the quals down into the component queries of the setop, but getting it
         * right seems nontrivial.  Work on this later.)
         *
-        * 2. If the subquery has a LIMIT clause we must not push down any quals,
-        * since that could change the set of rows returned.
+        * 2. If the subquery has a LIMIT clause or a DISTINCT ON clause, we must
+        * not push down any quals, since that could change the set of rows
+        * returned.  (Actually, we could push down quals into a DISTINCT ON
+        * subquery if they refer only to DISTINCT-ed output columns, but checking
+        * that seems more work than it's worth.  In any case, a plain DISTINCT is
+        * safe to push down past.)
         *
         * 3. We do not push down clauses that contain subselects, mainly because
         * 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,
         */
        if (subquery->setOperations == NULL &&
                subquery->limitOffset == NULL &&
-               subquery->limitCount == NULL)
+               subquery->limitCount == NULL &&
+               !has_distinct_on_clause(subquery))
        {
                /* OK to consider pushing down individual quals */
                List       *upperrestrictlist = NIL;
index 6ee962fd75c1f01b322775f853a69a34b1f26f93..5bb850e0da2f937ee6a7dd2bec698527c934d666 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.86 2001/06/19 22:39:11 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.87 2001/07/31 17:56:31 tgl Exp $
  *
  * HISTORY
  *       AUTHOR                        DATE                    MAJOR EVENT
@@ -728,6 +728,58 @@ pull_constant_clauses(List *quals, List **constantQual)
 }
 
 
+/*****************************************************************************
+ *             Tests on clauses of queries
+ *
+ * Possibly this code should go someplace else, since this isn't quite the
+ * same meaning of "clause" as is used elsewhere in this module.  But I can't
+ * think of a better place for it...
+ *****************************************************************************/
+
+/*
+ * Test whether a query uses DISTINCT ON, ie, has a distinct-list that is
+ * just a subset of the output columns.
+ */
+bool
+has_distinct_on_clause(Query *query)
+{
+       List   *targetList;
+
+       /* Is there a DISTINCT clause at all? */
+       if (query->distinctClause == NIL)
+               return false;
+       /*
+        * If the DISTINCT list contains all the nonjunk targetlist items,
+        * then it's a simple DISTINCT, else it's DISTINCT ON.  We do not
+        * require the lists to be in the same order (since the parser may
+        * have adjusted the DISTINCT clause ordering to agree with ORDER BY).
+        */
+       foreach(targetList, query->targetList)
+       {
+               TargetEntry *tle = (TargetEntry *) lfirst(targetList);
+               Index           ressortgroupref;
+               List       *distinctClause;
+
+               if (tle->resdom->resjunk)
+                       continue;
+               ressortgroupref = tle->resdom->ressortgroupref;
+               if (ressortgroupref == 0)
+                       return true;            /* definitely not in DISTINCT list */
+               foreach(distinctClause, query->distinctClause)
+               {
+                       SortClause *scl = (SortClause *) lfirst(distinctClause);
+
+                       if (scl->tleSortGroupRef == ressortgroupref)
+                               break;                  /* found TLE in DISTINCT */
+               }
+               if (distinctClause == NIL)
+                       return true;            /* this TLE is not in DISTINCT list */
+       }
+       /* It's a simple DISTINCT */
+       return false;
+}
+
+
 /*****************************************************************************
  *                                                                                                                                                      *
  *             General clause-manipulating routines                                                             *
@@ -735,7 +787,7 @@ pull_constant_clauses(List *quals, List **constantQual)
  *****************************************************************************/
 
 /*
- * clause_relids_vars
+ * clause_get_relids_vars
  *       Retrieves distinct relids and vars appearing within a clause.
  *
  * '*relids' is set to an integer list of all distinct "varno"s appearing
index afc2b63430bae8819fa19c611768dabb70145dda..bda842e794eff43dde1c8c9da0ed0f6de90105af 100644 (file)
@@ -3,7 +3,7 @@
  *                             back to source text
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.80 2001/07/16 05:06:59 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.81 2001/07/31 17:56:31 tgl Exp $
  *
  *       This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -119,7 +119,6 @@ static void get_utility_query_def(Query *query, deparse_context *context);
 static void get_basic_select_query(Query *query, deparse_context *context);
 static void get_setop_query(Node *setOp, Query *query,
                                deparse_context *context, bool toplevel);
-static bool simple_distinct(List *distinctClause, List *targetList);
 static void get_rule_sortgroupclause(SortClause *srt, List *tlist,
                                                                         bool force_colno,
                                                                         deparse_context *context);
@@ -1064,9 +1063,7 @@ get_basic_select_query(Query *query, deparse_context *context)
        /* Add the DISTINCT clause if given */
        if (query->distinctClause != NIL)
        {
-               if (simple_distinct(query->distinctClause, query->targetList))
-                       appendStringInfo(buf, " DISTINCT");
-               else
+               if (has_distinct_on_clause(query))
                {
                        appendStringInfo(buf, " DISTINCT ON (");
                        sep = "";
@@ -1081,6 +1078,8 @@ get_basic_select_query(Query *query, deparse_context *context)
                        }
                        appendStringInfo(buf, ")");
                }
+               else
+                       appendStringInfo(buf, " DISTINCT");
        }
 
        /* Then we tell what to select (the targetlist) */
@@ -1207,34 +1206,6 @@ get_setop_query(Node *setOp, Query *query, deparse_context *context,
        }
 }
 
-/*
- * Detect whether a DISTINCT list can be represented as just DISTINCT
- * or needs DISTINCT ON.  It's simple if it contains exactly the nonjunk
- * targetlist items.
- */
-static bool
-simple_distinct(List *distinctClause, List *targetList)
-{
-       while (targetList)
-       {
-               TargetEntry *tle = (TargetEntry *) lfirst(targetList);
-
-               if (!tle->resdom->resjunk)
-               {
-                       if (distinctClause == NIL)
-                               return false;
-                       if (((SortClause *) lfirst(distinctClause))->tleSortGroupRef !=
-                               tle->resdom->ressortgroupref)
-                               return false;
-                       distinctClause = lnext(distinctClause);
-               }
-               targetList = lnext(targetList);
-       }
-       if (distinctClause != NIL)
-               return false;
-       return true;
-}
-
 /*
  * Display a sort/group clause.
  */
index 79bb7a1359347cf357c000dc80cb643efb09228e..1ed5c25cd79f4984b7fdc2c7dfb160ea4cb68b9d 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: clauses.h,v 1.44 2001/05/20 20:28:20 tgl Exp $
+ * $Id: clauses.h,v 1.45 2001/07/31 17:56:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -51,6 +51,8 @@ extern bool contain_noncachable_functions(Node *clause);
 extern bool is_pseudo_constant_clause(Node *clause);
 extern List *pull_constant_clauses(List *quals, List **constantQual);
 
+extern bool has_distinct_on_clause(Query *query);
+
 extern void clause_get_relids_vars(Node *clause, Relids *relids, List **vars);
 extern int     NumRelids(Node *clause);
 extern void CommuteClause(Expr *clause);