From a4997254693057d4587d74222881b8e03580da2c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 16 Jun 2003 02:03:38 +0000 Subject: [PATCH] Allow GROUP BY, ORDER BY, DISTINCT targets to be unknown literals, silently resolving them to type TEXT. This is comparable to what we do when faced with UNKNOWN in CASE, UNION, and other contexts. It gets rid of this and related annoyances: select distinct f1, '' from int4_tbl; ERROR: Unable to identify an ordering operator '<' for type unknown This was discussed many moons ago, but no one got round to fixing it. --- src/backend/optimizer/plan/createplan.c | 4 +- src/backend/optimizer/plan/planner.c | 39 +++++++++---- src/backend/optimizer/prep/prepunion.c | 6 +- src/backend/parser/analyze.c | 8 ++- src/backend/parser/parse_clause.c | 78 +++++++++++++++++++------ src/include/parser/parse_clause.h | 9 ++- 6 files changed, 105 insertions(+), 39 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 273a80129e..81ee7962df 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.145 2003/06/15 22:51:45 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.146 2003/06/16 02:03:37 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -583,7 +583,7 @@ create_unique_plan(Query *root, UniquePath *best_path) { List *sortList; - sortList = addAllTargetsToSortList(NIL, my_tlist); + sortList = addAllTargetsToSortList(NULL, NIL, my_tlist, false); plan = (Plan *) make_sort_from_sortclauses(root, my_tlist, subplan, sortList); plan = (Plan *) make_unique(my_tlist, plan, sortList); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index fdb5519862..f55d0cad5d 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.154 2003/06/06 15:04:02 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.155 2003/06/16 02:03:37 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -894,6 +894,24 @@ grouping_planner(Query *parse, double tuple_fraction) if (parse->groupClause) { List *groupExprs; + double cheapest_path_rows; + int cheapest_path_width; + + /* + * Beware in this section of the possibility that + * cheapest_path->parent is NULL. This could happen if user + * does something silly like SELECT 'foo' GROUP BY 1; + */ + if (cheapest_path->parent) + { + cheapest_path_rows = cheapest_path->parent->rows; + cheapest_path_width = cheapest_path->parent->width; + } + else + { + cheapest_path_rows = 1; /* assume non-set result */ + cheapest_path_width = 100; /* arbitrary */ + } /* * Always estimate the number of groups. We can't do this until @@ -903,7 +921,7 @@ grouping_planner(Query *parse, double tuple_fraction) parse->targetList); dNumGroups = estimate_num_groups(parse, groupExprs, - cheapest_path->parent->rows); + cheapest_path_rows); /* Also want it as a long int --- but 'ware overflow! */ numGroups = (long) Min(dNumGroups, (double) LONG_MAX); @@ -936,8 +954,7 @@ grouping_planner(Query *parse, double tuple_fraction) * assume it is 100 bytes. Also set the overhead per hashtable * entry at 64 bytes. */ - int hashentrysize = cheapest_path->parent->width + 64 + - numAggs * 100; + int hashentrysize = cheapest_path_width + 64 + numAggs * 100; if (hashentrysize * dNumGroups <= SortMem * 1024L) { @@ -964,13 +981,13 @@ grouping_planner(Query *parse, double tuple_fraction) numGroupCols, dNumGroups, cheapest_path->startup_cost, cheapest_path->total_cost, - cheapest_path->parent->rows); + cheapest_path_rows); /* Result of hashed agg is always unsorted */ if (sort_pathkeys) cost_sort(&hashed_p, parse, sort_pathkeys, hashed_p.total_cost, dNumGroups, - cheapest_path->parent->width); + cheapest_path_width); if (sorted_path) { @@ -989,8 +1006,8 @@ grouping_planner(Query *parse, double tuple_fraction) { cost_sort(&sorted_p, parse, group_pathkeys, sorted_p.total_cost, - cheapest_path->parent->rows, - cheapest_path->parent->width); + cheapest_path_rows, + cheapest_path_width); current_pathkeys = group_pathkeys; } if (parse->hasAggs) @@ -999,13 +1016,13 @@ grouping_planner(Query *parse, double tuple_fraction) numGroupCols, dNumGroups, sorted_p.startup_cost, sorted_p.total_cost, - cheapest_path->parent->rows); + cheapest_path_rows); else cost_group(&sorted_p, parse, numGroupCols, dNumGroups, sorted_p.startup_cost, sorted_p.total_cost, - cheapest_path->parent->rows); + cheapest_path_rows); /* The Agg or Group node will preserve ordering */ if (sort_pathkeys && !pathkeys_contained_in(sort_pathkeys, @@ -1014,7 +1031,7 @@ grouping_planner(Query *parse, double tuple_fraction) cost_sort(&sorted_p, parse, sort_pathkeys, sorted_p.total_cost, dNumGroups, - cheapest_path->parent->width); + cheapest_path_width); } /* diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 86a52645fe..ac6e0f0f35 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -14,7 +14,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.95 2003/05/06 00:20:32 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.96 2003/06/16 02:03:37 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -239,7 +239,7 @@ generate_union_plan(SetOperationStmt *op, Query *parse, List *sortList; tlist = copyObject(tlist); - sortList = addAllTargetsToSortList(NIL, tlist); + sortList = addAllTargetsToSortList(NULL, NIL, tlist, false); plan = (Plan *) make_sort_from_sortclauses(parse, tlist, plan, sortList); plan = (Plan *) make_unique(tlist, plan, sortList); @@ -293,7 +293,7 @@ generate_nonunion_plan(SetOperationStmt *op, Query *parse, * correct output. */ tlist = copyObject(tlist); - sortList = addAllTargetsToSortList(NIL, tlist); + sortList = addAllTargetsToSortList(NULL, NIL, tlist, false); plan = (Plan *) make_sort_from_sortclauses(parse, tlist, plan, sortList); switch (op->op) { diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 95b209e4ac..3f4cb22cdf 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.274 2003/06/15 16:42:07 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.275 2003/06/16 02:03:37 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1793,7 +1793,8 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) */ qry->sortClause = transformSortClause(pstate, stmt->sortClause, - qry->targetList); + qry->targetList, + true /* fix unknowns */); qry->groupClause = transformGroupClause(pstate, stmt->groupClause, @@ -2002,7 +2003,8 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) qry->sortClause = transformSortClause(pstate, sortClause, - qry->targetList); + qry->targetList, + false /* no unknowns expected */); pstate->p_namespace = sv_namespace; pstate->p_rtable = sv_rtable; diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 2e2beabddc..4d08704da6 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.115 2003/06/15 16:42:07 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.116 2003/06/16 02:03:37 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -59,8 +59,9 @@ static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype, Var *l_colvar, Var *r_colvar); static TargetEntry *findTargetlistEntry(ParseState *pstate, Node *node, List *tlist, int clause); -static List *addTargetToSortList(TargetEntry *tle, List *sortlist, - List *targetlist, List *opname); +static List *addTargetToSortList(ParseState *pstate, TargetEntry *tle, + List *sortlist, List *targetlist, + List *opname, bool resolveUnknown); /* @@ -1133,6 +1134,7 @@ transformGroupClause(ParseState *pstate, List *grouplist, foreach(gl, grouplist) { TargetEntry *tle; + Oid restype; Oid ordering_op; GroupClause *grpcl; @@ -1143,6 +1145,19 @@ transformGroupClause(ParseState *pstate, List *grouplist, if (targetIsInSortList(tle, glist)) continue; + /* if tlist item is an UNKNOWN literal, change it to TEXT */ + restype = tle->resdom->restype; + + if (restype == UNKNOWNOID) + { + tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr, + restype, TEXTOID, + COERCION_IMPLICIT, + COERCE_IMPLICIT_CAST); + restype = tle->resdom->restype = TEXTOID; + tle->resdom->restypmod = -1; + } + /* * If the GROUP BY clause matches the ORDER BY clause, we want to * adopt the ordering operators from the latter rather than using @@ -1160,7 +1175,7 @@ transformGroupClause(ParseState *pstate, List *grouplist, } else { - ordering_op = ordering_oper_opid(tle->resdom->restype); + ordering_op = ordering_oper_opid(restype); sortClause = NIL; /* disregard ORDER BY once match fails */ } @@ -1180,7 +1195,8 @@ transformGroupClause(ParseState *pstate, List *grouplist, List * transformSortClause(ParseState *pstate, List *orderlist, - List *targetlist) + List *targetlist, + bool resolveUnknown) { List *sortlist = NIL; List *olitem; @@ -1193,8 +1209,9 @@ transformSortClause(ParseState *pstate, tle = findTargetlistEntry(pstate, sortby->node, targetlist, ORDER_CLAUSE); - sortlist = addTargetToSortList(tle, sortlist, targetlist, - sortby->useOp); + sortlist = addTargetToSortList(pstate, tle, + sortlist, targetlist, + sortby->useOp, resolveUnknown); } return sortlist; @@ -1232,7 +1249,10 @@ transformDistinctClause(ParseState *pstate, List *distinctlist, * the user's ORDER BY spec alone, and just add additional sort * keys to it to ensure that all targetlist items get sorted.) */ - *sortClause = addAllTargetsToSortList(*sortClause, targetlist); + *sortClause = addAllTargetsToSortList(pstate, + *sortClause, + targetlist, + true); /* * Now, DISTINCT list consists of all non-resjunk sortlist items. @@ -1291,8 +1311,9 @@ transformDistinctClause(ParseState *pstate, List *distinctlist, } else { - *sortClause = addTargetToSortList(tle, *sortClause, - targetlist, NIL); + *sortClause = addTargetToSortList(pstate, tle, + *sortClause, targetlist, + NIL, true); /* * Probably, the tle should always have been added at the @@ -1323,10 +1344,13 @@ transformDistinctClause(ParseState *pstate, List *distinctlist, * ORDER BY list, adding the not-yet-sorted ones to the end of the list. * This is typically used to help implement SELECT DISTINCT. * + * See addTargetToSortList for info about pstate and resolveUnknown inputs. + * * Returns the updated ORDER BY list. */ List * -addAllTargetsToSortList(List *sortlist, List *targetlist) +addAllTargetsToSortList(ParseState *pstate, List *sortlist, + List *targetlist, bool resolveUnknown) { List *i; @@ -1335,7 +1359,9 @@ addAllTargetsToSortList(List *sortlist, List *targetlist) TargetEntry *tle = (TargetEntry *) lfirst(i); if (!tle->resdom->resjunk) - sortlist = addTargetToSortList(tle, sortlist, targetlist, NIL); + sortlist = addTargetToSortList(pstate, tle, + sortlist, targetlist, + NIL, resolveUnknown); } return sortlist; } @@ -1346,26 +1372,44 @@ addAllTargetsToSortList(List *sortlist, List *targetlist) * add it to the end of the list, using the sortop with given name * or the default sort operator if opname == NIL. * + * If resolveUnknown is TRUE, convert TLEs of type UNKNOWN to TEXT. If not, + * do nothing (which implies the search for a sort operator will fail). + * pstate should be provided if resolveUnknown is TRUE, but can be NULL + * otherwise. + * * Returns the updated ORDER BY list. */ static List * -addTargetToSortList(TargetEntry *tle, List *sortlist, List *targetlist, - List *opname) +addTargetToSortList(ParseState *pstate, TargetEntry *tle, + List *sortlist, List *targetlist, + List *opname, bool resolveUnknown) { /* avoid making duplicate sortlist entries */ if (!targetIsInSortList(tle, sortlist)) { SortClause *sortcl = makeNode(SortClause); + Oid restype = tle->resdom->restype; + + /* if tlist item is an UNKNOWN literal, change it to TEXT */ + if (restype == UNKNOWNOID && resolveUnknown) + { + tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr, + restype, TEXTOID, + COERCION_IMPLICIT, + COERCE_IMPLICIT_CAST); + restype = tle->resdom->restype = TEXTOID; + tle->resdom->restypmod = -1; + } sortcl->tleSortGroupRef = assignSortGroupRef(tle, targetlist); if (opname) sortcl->sortop = compatible_oper_opid(opname, - tle->resdom->restype, - tle->resdom->restype, + restype, + restype, false); else - sortcl->sortop = ordering_oper_opid(tle->resdom->restype); + sortcl->sortop = ordering_oper_opid(restype); sortlist = lappend(sortlist, sortcl); } diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h index 12ab7317d9..c2a3b2dc77 100644 --- a/src/include/parser/parse_clause.h +++ b/src/include/parser/parse_clause.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: parse_clause.h,v 1.31 2003/06/15 16:42:08 tgl Exp $ + * $Id: parse_clause.h,v 1.32 2003/06/16 02:03:38 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -20,15 +20,18 @@ extern void transformFromClause(ParseState *pstate, List *frmList); extern int setTargetTable(ParseState *pstate, RangeVar *relation, bool inh, bool alsoSource); extern bool interpretInhOption(InhOption inhOpt); + extern Node *transformWhereClause(ParseState *pstate, Node *where); extern List *transformGroupClause(ParseState *pstate, List *grouplist, List *targetlist, List *sortClause); extern List *transformSortClause(ParseState *pstate, List *orderlist, - List *targetlist); + List *targetlist, bool resolveUnknown); extern List *transformDistinctClause(ParseState *pstate, List *distinctlist, List *targetlist, List **sortClause); -extern List *addAllTargetsToSortList(List *sortlist, List *targetlist); +extern List *addAllTargetsToSortList(ParseState *pstate, + List *sortlist, List *targetlist, + bool resolveUnknown); extern Index assignSortGroupRef(TargetEntry *tle, List *tlist); extern bool targetIsInSortList(TargetEntry *tle, List *sortList);