Commit 7ace43e0 authored by Tom Lane's avatar Tom Lane

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

from index, since the aggregates ignore NULLs.
parent 2e7a6889
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
* *
* *
* IDENTIFICATION * 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, ...@@ -73,7 +73,6 @@ static void fix_indxqual_sublist(List *indexqual, IndexOptInfo *index,
static Node *fix_indxqual_operand(Node *node, IndexOptInfo *index, static Node *fix_indxqual_operand(Node *node, IndexOptInfo *index,
Oid *opclass); Oid *opclass);
static List *get_switched_clauses(List *clauses, Relids outerrelids); 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_path_costsize(Plan *dest, Path *src);
static void copy_plan_costsize(Plan *dest, Plan *src); static void copy_plan_costsize(Plan *dest, Plan *src);
static SeqScan *make_seqscan(List *qptlist, List *qpqual, Index scanrelid); static SeqScan *make_seqscan(List *qptlist, List *qpqual, Index scanrelid);
...@@ -1417,7 +1416,7 @@ get_switched_clauses(List *clauses, Relids outerrelids) ...@@ -1417,7 +1416,7 @@ get_switched_clauses(List *clauses, Relids outerrelids)
* For now, we just move any quals that contain SubPlan references (but not * For now, we just move any quals that contain SubPlan references (but not
* InitPlan references) to the end of the list. * InitPlan references) to the end of the list.
*/ */
static List * List *
order_qual_clauses(Query *root, List *clauses) order_qual_clauses(Query *root, List *clauses)
{ {
List *nosubplans; List *nosubplans;
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * 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) ...@@ -185,6 +185,8 @@ optimize_minmax_aggregates(Query *root, List *tlist, Path *best_path)
{ {
Assert(((ResultPath *) best_path)->subpath != NULL); Assert(((ResultPath *) best_path)->subpath != NULL);
constant_quals = ((ResultPath *) best_path)->constantqual; constant_quals = ((ResultPath *) best_path)->constantqual;
/* no need to do this more than once: */
constant_quals = order_qual_clauses(root, constant_quals);
} }
else else
constant_quals = NIL; constant_quals = NIL;
...@@ -438,10 +440,10 @@ static void ...@@ -438,10 +440,10 @@ static void
make_agg_subplan(Query *root, MinMaxAggInfo *info, List *constant_quals) make_agg_subplan(Query *root, MinMaxAggInfo *info, List *constant_quals)
{ {
Query *subquery; Query *subquery;
Path *path;
Plan *plan; Plan *plan;
TargetEntry *tle; TargetEntry *tle;
SortClause *sortcl; SortClause *sortcl;
NullTest *ntest;
/* /*
* Generate a suitably modified Query node. Much of the work here is * 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) ...@@ -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 * 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 * 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, * 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) plan->targetlist = copyObject(subquery->targetList);
path = (Path *) create_result_path(NULL,
path,
copyObject(constant_quals));
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, plan = (Plan *) make_limit(plan,
subquery->limitOffset, subquery->limitOffset,
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * 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, ...@@ -40,6 +40,7 @@ extern Sort *make_sort_from_sortclauses(Query *root, List *sortcls,
Plan *lefttree); Plan *lefttree);
extern Sort *make_sort_from_groupcols(Query *root, List *groupcls, extern Sort *make_sort_from_groupcols(Query *root, List *groupcls,
AttrNumber *grpColIdx, Plan *lefttree); AttrNumber *grpColIdx, Plan *lefttree);
extern List *order_qual_clauses(Query *root, List *clauses);
extern Agg *make_agg(Query *root, List *tlist, List *qual, extern Agg *make_agg(Query *root, List *tlist, List *qual,
AggStrategy aggstrategy, AggStrategy aggstrategy,
int numGroupCols, AttrNumber *grpColIdx, int numGroupCols, AttrNumber *grpColIdx,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment