Anthony Wood ([EMAIL PROTECTED]) writes:
> [ SELECT DISTINCT ON in a subquery-in-FROM misbehaves ]

Here's the patch against 7.1.2 to fix this problem.  This also fixes a
related problem noted a few days ago, that outer WHERE clauses shouldn't
be pushed down into a sub-select that has a LIMIT clause.

                        regards, tom lane

*** src/backend/optimizer/path/allpaths.c.orig  Wed Mar 21 22:59:34 2001
--- src/backend/optimizer/path/allpaths.c       Tue Jul 31 14:05:05 2001
***************
*** 125,135 ****
                         * Non-pushed-down clauses will get evaluated as qpquals of
                         * the SubqueryScan node.
                         *
                         * XXX Are there any cases where we want to make a policy
                         * decision not to push down, because it'd result in a worse
                         * plan?
                         */
!                       if (rte->subquery->setOperations == NULL)
                        {
                                /* OK to consider pushing down individual quals */
                                List       *upperrestrictlist = NIL;
--- 125,141 ----
                         * Non-pushed-down clauses will get evaluated as qpquals of
                         * the SubqueryScan node.
                         *
+                        * We can't push down into subqueries with LIMIT or DISTINCT ON
+                        * clauses, either.
+                        *
                         * XXX Are there any cases where we want to make a policy
                         * decision not to push down, because it'd result in a worse
                         * plan?
                         */
!                       if (rte->subquery->setOperations == NULL &&
!                               rte->subquery->limitOffset == NULL &&
!                               rte->subquery->limitCount == NULL &&
!                               !has_distinct_on_clause(rte->subquery))
                        {
                                /* OK to consider pushing down individual quals */
                                List       *upperrestrictlist = NIL;
*** src/backend/optimizer/util/clauses.c.orig   Tue Mar 27 12:12:34 2001
--- src/backend/optimizer/util/clauses.c        Tue Jul 31 14:05:01 2001
***************
*** 730,742 ****
  
  
  /*****************************************************************************
   *                                                                                   
                                                                  *
   *            General clause-manipulating routines                                   
                          *
   *                                                                                   
                                                                  *
   *****************************************************************************/
  
  /*
!  * clause_relids_vars
   *      Retrieves distinct relids and vars appearing within a clause.
   *
   * '*relids' is set to an integer list of all distinct "varno"s appearing
--- 730,794 ----
  
  
  /*****************************************************************************
+  *            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                                   
                          *
   *                                                                                   
                                                                  *
   *****************************************************************************/
  
  /*
!  * 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
*** src/backend/utils/adt/ruleutils.c.orig      Wed Apr 18 13:04:24 2001
--- src/backend/utils/adt/ruleutils.c   Tue Jul 31 14:04:51 2001
***************
*** 119,125 ****
  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);
--- 119,124 ----
***************
*** 1006,1014 ****
        /* Add the DISTINCT clause if given */
        if (query->distinctClause != NIL)
        {
!               if (simple_distinct(query->distinctClause, query->targetList))
!                       appendStringInfo(buf, " DISTINCT");
!               else
                {
                        appendStringInfo(buf, " DISTINCT ON (");
                        sep = "";
--- 1005,1011 ----
        /* Add the DISTINCT clause if given */
        if (query->distinctClause != NIL)
        {
!               if (has_distinct_on_clause(query))
                {
                        appendStringInfo(buf, " DISTINCT ON (");
                        sep = "";
***************
*** 1023,1028 ****
--- 1020,1027 ----
                        }
                        appendStringInfo(buf, ")");
                }
+               else
+                       appendStringInfo(buf, " DISTINCT");
        }
  
        /* Then we tell what to select (the targetlist) */
***************
*** 1147,1180 ****
                elog(ERROR, "get_setop_query: unexpected node %d",
                         (int) nodeTag(setOp));
        }
- }
- 
- /*
-  * 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;
  }
  
  /*
--- 1146,1151 ----
*** src/include/optimizer/clauses.h.orig        Wed Mar 21 23:00:53 2001
--- src/include/optimizer/clauses.h     Tue Jul 31 14:04:35 2001
***************
*** 59,64 ****
--- 59,66 ----
  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 get_relattval(Node *clause, int targetrelid,

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

Reply via email to