Fix oversight in MIN/MAX optimization: must not return NULL entries

from index, since the aggregates ignore NULLs.

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 41e2edceb26511d53b7b908d19b8350a91038cc8..6f378c76dd895e2861d2d061d044647165e84f3c 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.178 2005/04/06 16:34:05 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.179 2005/04/12 05:11:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -73,7 +73,6 @@ static void fix_indxqual_sublist(List *indexqual, IndexOptInfo *index,
 static Node *fix_indxqual_operand(Node *node, IndexOptInfo *index,
                                  Oid *opclass);
 static List *get_switched_clauses(List *clauses, Relids outerrelids);
-static List *order_qual_clauses(Query *root, List *clauses);
 static void copy_path_costsize(Plan *dest, Path *src);
 static void copy_plan_costsize(Plan *dest, Plan *src);
 static SeqScan *make_seqscan(List *qptlist, List *qpqual, Index scanrelid);
@@ -1417,7 +1416,7 @@ get_switched_clauses(List *clauses, Relids outerrelids)
  * For now, we just move any quals that contain SubPlan references (but not
  * InitPlan references) to the end of the list.
  */
-static List *
+List *
 order_qual_clauses(Query *root, List *clauses)
 {
    List       *nosubplans;

diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index b80f2c6900b7ccf6b03f0e24336dae23acccf7b5..9c3b151f2010531cbedf7d8e447c280a689c5abf 100644 (file)
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.2 2005/04/12 04:26:24 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.3 2005/04/12 05:11:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -185,6 +185,8 @@ optimize_minmax_aggregates(Query *root, List *tlist, Path *best_path)
    {
        Assert(((ResultPath *) best_path)->subpath != NULL);
        constant_quals = ((ResultPath *) best_path)->constantqual;
+       /* no need to do this more than once: */
+       constant_quals = order_qual_clauses(root, constant_quals);
    }
    else
        constant_quals = NIL;
@@ -438,10 +440,10 @@ static void
 make_agg_subplan(Query *root, MinMaxAggInfo *info, List *constant_quals)
 {
    Query      *subquery;
-   Path       *path;
    Plan       *plan;
    TargetEntry *tle;
    SortClause *sortcl;
+   NullTest   *ntest;
 
    /*
     * Generate a suitably modified Query node.  Much of the work here is
@@ -482,18 +484,30 @@ make_agg_subplan(Query *root, MinMaxAggInfo *info, List *constant_quals)
     * Generate the plan for the subquery.  We already have a Path for
     * the basic indexscan, but we have to convert it to a Plan and
     * attach a LIMIT node above it.  We might need a gating Result, too,
-    * which is most easily added at the Path stage.
+    * to handle any non-variable qual clauses.
+    *
+    * Also we must add a "WHERE foo IS NOT NULL" restriction to the
+    * indexscan, to be sure we don't return a NULL, which'd be contrary
+    * to the standard behavior of MIN/MAX.  XXX ideally this should be
+    * done earlier, so that the selectivity of the restriction could be
+    * included in our cost estimates.  But that looks painful, and in
+    * most cases the fraction of NULLs isn't high enough to change the
+    * decision.
     */
-   path = (Path *) info->path;
+   plan = create_plan(subquery, (Path *) info->path);
 
-   if (constant_quals)
-       path = (Path *) create_result_path(NULL,
-                                          path,
-                                          copyObject(constant_quals));
+   plan->targetlist = copyObject(subquery->targetList);
 
-   plan = create_plan(subquery, path);
+   ntest = makeNode(NullTest);
+   ntest->nulltesttype = IS_NOT_NULL;
+   ntest->arg = copyObject(info->target);
 
-   plan->targetlist = copyObject(subquery->targetList);
+   plan->qual = lappend(plan->qual, ntest);
+
+   if (constant_quals)
+       plan = (Plan *) make_result(copyObject(plan->targetlist),
+                                   copyObject(constant_quals),
+                                   plan);
 
    plan = (Plan *) make_limit(plan, 
                               subquery->limitOffset,

diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index f2a48e77c9e3bda6726ec764f43d1abc82997c2f..4f39cc3527b61e2c630a7ece7133aa2f11bc4241 100644 (file)
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/planmain.h,v 1.81 2005/04/11 23:06:56 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/planmain.h,v 1.82 2005/04/12 05:11:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -40,6 +40,7 @@ extern Sort *make_sort_from_sortclauses(Query *root, List *sortcls,
                           Plan *lefttree);
 extern Sort *make_sort_from_groupcols(Query *root, List *groupcls,
                         AttrNumber *grpColIdx, Plan *lefttree);
+extern List *order_qual_clauses(Query *root, List *clauses);
 extern Agg *make_agg(Query *root, List *tlist, List *qual,
         AggStrategy aggstrategy,
         int numGroupCols, AttrNumber *grpColIdx,