Skip to content

Commit a499725

Browse files
tglsfdctglsfdc
committed
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.
1 parent cb02610 commit a499725

File tree

6 files changed

+105
-39
lines changed

6 files changed

+105
-39
lines changed

‎src/backend/optimizer/plan/createplan.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.145 2003/06/15 22:51:45 tgl Exp $
13+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.146 2003/06/16 02:03:37 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -583,7 +583,7 @@ create_unique_plan(Query *root, UniquePath *best_path)
583583
{
584584
List *sortList;
585585

586-
sortList = addAllTargetsToSortList(NIL, my_tlist);
586+
sortList = addAllTargetsToSortList(NULL, NIL, my_tlist, false);
587587
plan = (Plan *) make_sort_from_sortclauses(root, my_tlist,
588588
subplan, sortList);
589589
plan = (Plan *) make_unique(my_tlist, plan, sortList);

‎src/backend/optimizer/plan/planner.c‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.154 2003/06/06 15:04:02 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.155 2003/06/16 02:03:37 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -894,6 +894,24 @@ grouping_planner(Query *parse, double tuple_fraction)
894894
if (parse->groupClause)
895895
{
896896
List *groupExprs;
897+
double cheapest_path_rows;
898+
int cheapest_path_width;
899+
900+
/*
901+
* Beware in this section of the possibility that
902+
* cheapest_path->parent is NULL. This could happen if user
903+
* does something silly like SELECT 'foo' GROUP BY 1;
904+
*/
905+
if (cheapest_path->parent)
906+
{
907+
cheapest_path_rows = cheapest_path->parent->rows;
908+
cheapest_path_width = cheapest_path->parent->width;
909+
}
910+
else
911+
{
912+
cheapest_path_rows = 1; /* assume non-set result */
913+
cheapest_path_width = 100; /* arbitrary */
914+
}
897915

898916
/*
899917
* Always estimate the number of groups. We can't do this until
@@ -903,7 +921,7 @@ grouping_planner(Query *parse, double tuple_fraction)
903921
parse->targetList);
904922
dNumGroups = estimate_num_groups(parse,
905923
groupExprs,
906-
cheapest_path->parent->rows);
924+
cheapest_path_rows);
907925
/* Also want it as a long int --- but 'ware overflow! */
908926
numGroups = (long) Min(dNumGroups, (double) LONG_MAX);
909927

@@ -936,8 +954,7 @@ grouping_planner(Query *parse, double tuple_fraction)
936954
* assume it is 100 bytes. Also set the overhead per hashtable
937955
* entry at 64 bytes.
938956
*/
939-
int hashentrysize = cheapest_path->parent->width + 64 +
940-
numAggs * 100;
957+
int hashentrysize = cheapest_path_width + 64 + numAggs * 100;
941958

942959
if (hashentrysize * dNumGroups <= SortMem * 1024L)
943960
{
@@ -964,13 +981,13 @@ grouping_planner(Query *parse, double tuple_fraction)
964981
numGroupCols, dNumGroups,
965982
cheapest_path->startup_cost,
966983
cheapest_path->total_cost,
967-
cheapest_path->parent->rows);
984+
cheapest_path_rows);
968985
/* Result of hashed agg is always unsorted */
969986
if (sort_pathkeys)
970987
cost_sort(&hashed_p, parse, sort_pathkeys,
971988
hashed_p.total_cost,
972989
dNumGroups,
973-
cheapest_path->parent->width);
990+
cheapest_path_width);
974991

975992
if (sorted_path)
976993
{
@@ -989,8 +1006,8 @@ grouping_planner(Query *parse, double tuple_fraction)
9891006
{
9901007
cost_sort(&sorted_p, parse, group_pathkeys,
9911008
sorted_p.total_cost,
992-
cheapest_path->parent->rows,
993-
cheapest_path->parent->width);
1009+
cheapest_path_rows,
1010+
cheapest_path_width);
9941011
current_pathkeys = group_pathkeys;
9951012
}
9961013
if (parse->hasAggs)
@@ -999,13 +1016,13 @@ grouping_planner(Query *parse, double tuple_fraction)
9991016
numGroupCols, dNumGroups,
10001017
sorted_p.startup_cost,
10011018
sorted_p.total_cost,
1002-
cheapest_path->parent->rows);
1019+
cheapest_path_rows);
10031020
else
10041021
cost_group(&sorted_p, parse,
10051022
numGroupCols, dNumGroups,
10061023
sorted_p.startup_cost,
10071024
sorted_p.total_cost,
1008-
cheapest_path->parent->rows);
1025+
cheapest_path_rows);
10091026
/* The Agg or Group node will preserve ordering */
10101027
if (sort_pathkeys &&
10111028
!pathkeys_contained_in(sort_pathkeys,
@@ -1014,7 +1031,7 @@ grouping_planner(Query *parse, double tuple_fraction)
10141031
cost_sort(&sorted_p, parse, sort_pathkeys,
10151032
sorted_p.total_cost,
10161033
dNumGroups,
1017-
cheapest_path->parent->width);
1034+
cheapest_path_width);
10181035
}
10191036

10201037
/*

‎src/backend/optimizer/prep/prepunion.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.95 2003/05/06 00:20:32 tgl Exp $
17+
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.96 2003/06/16 02:03:37 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -239,7 +239,7 @@ generate_union_plan(SetOperationStmt *op, Query *parse,
239239
List *sortList;
240240

241241
tlist = copyObject(tlist);
242-
sortList = addAllTargetsToSortList(NIL, tlist);
242+
sortList = addAllTargetsToSortList(NULL, NIL, tlist, false);
243243
plan = (Plan *) make_sort_from_sortclauses(parse, tlist,
244244
plan, sortList);
245245
plan = (Plan *) make_unique(tlist, plan, sortList);
@@ -293,7 +293,7 @@ generate_nonunion_plan(SetOperationStmt *op, Query *parse,
293293
* correct output.
294294
*/
295295
tlist = copyObject(tlist);
296-
sortList = addAllTargetsToSortList(NIL, tlist);
296+
sortList = addAllTargetsToSortList(NULL, NIL, tlist, false);
297297
plan = (Plan *) make_sort_from_sortclauses(parse, tlist, plan, sortList);
298298
switch (op->op)
299299
{

‎src/backend/parser/analyze.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.274 2003/06/15 16:42:07 tgl Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.275 2003/06/16 02:03:37 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -1793,7 +1793,8 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
17931793
*/
17941794
qry->sortClause = transformSortClause(pstate,
17951795
stmt->sortClause,
1796-
qry->targetList);
1796+
qry->targetList,
1797+
true /* fix unknowns */);
17971798

17981799
qry->groupClause = transformGroupClause(pstate,
17991800
stmt->groupClause,
@@ -2002,7 +2003,8 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
20022003

20032004
qry->sortClause = transformSortClause(pstate,
20042005
sortClause,
2005-
qry->targetList);
2006+
qry->targetList,
2007+
false /* no unknowns expected */);
20062008

20072009
pstate->p_namespace = sv_namespace;
20082010
pstate->p_rtable = sv_rtable;

‎src/backend/parser/parse_clause.c‎

Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.115 2003/06/15 16:42:07 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.116 2003/06/16 02:03:37 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -59,8 +59,9 @@ static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype,
5959
Var *l_colvar, Var *r_colvar);
6060
static TargetEntry *findTargetlistEntry(ParseState *pstate, Node *node,
6161
List *tlist, int clause);
62-
static List *addTargetToSortList(TargetEntry *tle, List *sortlist,
63-
List *targetlist, List *opname);
62+
static List *addTargetToSortList(ParseState *pstate, TargetEntry *tle,
63+
List *sortlist, List *targetlist,
64+
List *opname, bool resolveUnknown);
6465

6566

6667
/*
@@ -1133,6 +1134,7 @@ transformGroupClause(ParseState *pstate, List *grouplist,
11331134
foreach(gl, grouplist)
11341135
{
11351136
TargetEntry *tle;
1137+
Oid restype;
11361138
Oid ordering_op;
11371139
GroupClause *grpcl;
11381140

@@ -1143,6 +1145,19 @@ transformGroupClause(ParseState *pstate, List *grouplist,
11431145
if (targetIsInSortList(tle, glist))
11441146
continue;
11451147

1148+
/* if tlist item is an UNKNOWN literal, change it to TEXT */
1149+
restype = tle->resdom->restype;
1150+
1151+
if (restype == UNKNOWNOID)
1152+
{
1153+
tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr,
1154+
restype, TEXTOID,
1155+
COERCION_IMPLICIT,
1156+
COERCE_IMPLICIT_CAST);
1157+
restype = tle->resdom->restype = TEXTOID;
1158+
tle->resdom->restypmod = -1;
1159+
}
1160+
11461161
/*
11471162
* If the GROUP BY clause matches the ORDER BY clause, we want to
11481163
* adopt the ordering operators from the latter rather than using
@@ -1160,7 +1175,7 @@ transformGroupClause(ParseState *pstate, List *grouplist,
11601175
}
11611176
else
11621177
{
1163-
ordering_op = ordering_oper_opid(tle->resdom->restype);
1178+
ordering_op = ordering_oper_opid(restype);
11641179
sortClause = NIL; /* disregard ORDER BY once match fails */
11651180
}
11661181

@@ -1180,7 +1195,8 @@ transformGroupClause(ParseState *pstate, List *grouplist,
11801195
List *
11811196
transformSortClause(ParseState *pstate,
11821197
List *orderlist,
1183-
List *targetlist)
1198+
List *targetlist,
1199+
bool resolveUnknown)
11841200
{
11851201
List *sortlist = NIL;
11861202
List *olitem;
@@ -1193,8 +1209,9 @@ transformSortClause(ParseState *pstate,
11931209
tle = findTargetlistEntry(pstate, sortby->node,
11941210
targetlist, ORDER_CLAUSE);
11951211

1196-
sortlist = addTargetToSortList(tle, sortlist, targetlist,
1197-
sortby->useOp);
1212+
sortlist = addTargetToSortList(pstate, tle,
1213+
sortlist, targetlist,
1214+
sortby->useOp, resolveUnknown);
11981215
}
11991216

12001217
return sortlist;
@@ -1232,7 +1249,10 @@ transformDistinctClause(ParseState *pstate, List *distinctlist,
12321249
* the user's ORDER BY spec alone, and just add additional sort
12331250
* keys to it to ensure that all targetlist items get sorted.)
12341251
*/
1235-
*sortClause = addAllTargetsToSortList(*sortClause, targetlist);
1252+
*sortClause = addAllTargetsToSortList(pstate,
1253+
*sortClause,
1254+
targetlist,
1255+
true);
12361256

12371257
/*
12381258
* Now, DISTINCT list consists of all non-resjunk sortlist items.
@@ -1291,8 +1311,9 @@ transformDistinctClause(ParseState *pstate, List *distinctlist,
12911311
}
12921312
else
12931313
{
1294-
*sortClause = addTargetToSortList(tle, *sortClause,
1295-
targetlist, NIL);
1314+
*sortClause = addTargetToSortList(pstate, tle,
1315+
*sortClause, targetlist,
1316+
NIL, true);
12961317

12971318
/*
12981319
* Probably, the tle should always have been added at the
@@ -1323,10 +1344,13 @@ transformDistinctClause(ParseState *pstate, List *distinctlist,
13231344
* ORDER BY list, adding the not-yet-sorted ones to the end of the list.
13241345
* This is typically used to help implement SELECT DISTINCT.
13251346
*
1347+
* See addTargetToSortList for info about pstate and resolveUnknown inputs.
1348+
*
13261349
* Returns the updated ORDER BY list.
13271350
*/
13281351
List *
1329-
addAllTargetsToSortList(List *sortlist, List *targetlist)
1352+
addAllTargetsToSortList(ParseState *pstate, List *sortlist,
1353+
List *targetlist, bool resolveUnknown)
13301354
{
13311355
List *i;
13321356

@@ -1335,7 +1359,9 @@ addAllTargetsToSortList(List *sortlist, List *targetlist)
13351359
TargetEntry *tle = (TargetEntry *) lfirst(i);
13361360

13371361
if (!tle->resdom->resjunk)
1338-
sortlist = addTargetToSortList(tle, sortlist, targetlist, NIL);
1362+
sortlist = addTargetToSortList(pstate, tle,
1363+
sortlist, targetlist,
1364+
NIL, resolveUnknown);
13391365
}
13401366
return sortlist;
13411367
}
@@ -1346,26 +1372,44 @@ addAllTargetsToSortList(List *sortlist, List *targetlist)
13461372
* add it to the end of the list, using the sortop with given name
13471373
* or the default sort operator if opname == NIL.
13481374
*
1375+
* If resolveUnknown is TRUE, convert TLEs of type UNKNOWN to TEXT. If not,
1376+
* do nothing (which implies the search for a sort operator will fail).
1377+
* pstate should be provided if resolveUnknown is TRUE, but can be NULL
1378+
* otherwise.
1379+
*
13491380
* Returns the updated ORDER BY list.
13501381
*/
13511382
static List *
1352-
addTargetToSortList(TargetEntry *tle, List *sortlist, List *targetlist,
1353-
List *opname)
1383+
addTargetToSortList(ParseState *pstate, TargetEntry *tle,
1384+
List *sortlist, List *targetlist,
1385+
List *opname, bool resolveUnknown)
13541386
{
13551387
/* avoid making duplicate sortlist entries */
13561388
if (!targetIsInSortList(tle, sortlist))
13571389
{
13581390
SortClause *sortcl = makeNode(SortClause);
1391+
Oid restype = tle->resdom->restype;
1392+
1393+
/* if tlist item is an UNKNOWN literal, change it to TEXT */
1394+
if (restype == UNKNOWNOID && resolveUnknown)
1395+
{
1396+
tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr,
1397+
restype, TEXTOID,
1398+
COERCION_IMPLICIT,
1399+
COERCE_IMPLICIT_CAST);
1400+
restype = tle->resdom->restype = TEXTOID;
1401+
tle->resdom->restypmod = -1;
1402+
}
13591403

13601404
sortcl->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
13611405

13621406
if (opname)
13631407
sortcl->sortop = compatible_oper_opid(opname,
1364-
tle->resdom->restype,
1365-
tle->resdom->restype,
1408+
restype,
1409+
restype,
13661410
false);
13671411
else
1368-
sortcl->sortop = ordering_oper_opid(tle->resdom->restype);
1412+
sortcl->sortop = ordering_oper_opid(restype);
13691413

13701414
sortlist = lappend(sortlist, sortcl);
13711415
}

‎src/include/parser/parse_clause.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: parse_clause.h,v 1.31 2003/06/15 16:42:08 tgl Exp $
10+
* $Id: parse_clause.h,v 1.32 2003/06/16 02:03:38 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -20,15 +20,18 @@ extern void transformFromClause(ParseState *pstate, List *frmList);
2020
extern int setTargetTable(ParseState *pstate, RangeVar *relation,
2121
bool inh, bool alsoSource);
2222
extern bool interpretInhOption(InhOption inhOpt);
23+
2324
extern Node *transformWhereClause(ParseState *pstate, Node *where);
2425
extern List *transformGroupClause(ParseState *pstate, List *grouplist,
2526
List *targetlist, List *sortClause);
2627
extern List *transformSortClause(ParseState *pstate, List *orderlist,
27-
List *targetlist);
28+
List *targetlist, bool resolveUnknown);
2829
extern List *transformDistinctClause(ParseState *pstate, List *distinctlist,
2930
List *targetlist, List **sortClause);
3031

31-
extern List *addAllTargetsToSortList(List *sortlist, List *targetlist);
32+
extern List *addAllTargetsToSortList(ParseState *pstate,
33+
List *sortlist, List *targetlist,
34+
bool resolveUnknown);
3235
extern Index assignSortGroupRef(TargetEntry *tle, List *tlist);
3336
extern bool targetIsInSortList(TargetEntry *tle, List *sortList);
3437

0 commit comments

Comments
 (0)