From 7ace43e0c2c1e00c3c5e87154a1e5ec61390285d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 12 Apr 2005 05:11:28 +0000 Subject: [PATCH] Fix oversight in MIN/MAX optimization: must not return NULL entries from index, since the aggregates ignore NULLs. --- src/backend/optimizer/plan/createplan.c | 5 ++--- src/backend/optimizer/plan/planagg.c | 34 +++++++++++++++++++++++---------- src/include/optimizer/planmain.h | 3 ++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 41e2edceb2..6f378c76dd 100644 --- 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 b80f2c6900..9c3b151f20 100644 --- 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 f2a48e77c9..4f39cc3527 100644 --- 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, -- 2.11.0