Skip to content

Commit f68f119

Browse files
tglsfdctglsfdc
committed
Tighten selection of equality and ordering operators for grouping
operations: make sure we use operators that are compatible, as determined by a mergejoin link in pg_operator. Also, add code to planner to ensure we don't try to use hashed grouping when the grouping operators aren't marked hashable.
1 parent 851a4c4 commit f68f119

File tree

8 files changed

+165
-79
lines changed

8 files changed

+165
-79
lines changed

‎src/backend/commands/analyze.c

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.50 2002/11/13 00:39:46 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.51 2002/11/29 21:39:10 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -402,10 +402,7 @@ examine_attribute(Relation onerel, int attnum)
402402
return NULL;
403403

404404
/* If column has no "=" operator, we can't do much of anything */
405-
func_operator = compatible_oper(makeList1(makeString("=")),
406-
attr->atttypid,
407-
attr->atttypid,
408-
true);
405+
func_operator = equality_oper(attr->atttypid, true);
409406
if (func_operator != NULL)
410407
{
411408
oprrest = ((Form_pg_operator) GETSTRUCT(func_operator))->oprrest;
@@ -443,10 +440,7 @@ examine_attribute(Relation onerel, int attnum)
443440
stats->attr->attstattarget = default_statistics_target;
444441

445442
/* Is there a "<" operator with suitable semantics? */
446-
func_operator = compatible_oper(makeList1(makeString("<")),
447-
attr->atttypid,
448-
attr->atttypid,
449-
true);
443+
func_operator = ordering_oper(attr->atttypid, true);
450444
if (func_operator != NULL)
451445
{
452446
oprrest = ((Form_pg_operator) GETSTRUCT(func_operator))->oprrest;

‎src/backend/executor/nodeAgg.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
* Portions Copyright (c) 1994, Regents of the University of California
4646
*
4747
* IDENTIFICATION
48-
* $Header: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v 1.96 2002/11/19 23:21:57 tgl Exp $
48+
* $Header: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v 1.97 2002/11/29 21:39:11 tgl Exp $
4949
*
5050
*-------------------------------------------------------------------------
5151
*/
@@ -1321,14 +1321,9 @@ ExecInitAgg(Agg *node, EState *estate, Plan *parent)
13211321
&peraggstate->inputtypeLen,
13221322
&peraggstate->inputtypeByVal);
13231323

1324-
eq_function = compatible_oper_funcid(makeList1(makeString("=")),
1325-
inputType, inputType,
1326-
true);
1327-
if (!OidIsValid(eq_function))
1328-
elog(ERROR, "Unable to identify an equality operator for type %s",
1329-
format_type_be(inputType));
1324+
eq_function = equality_oper_funcid(inputType);
13301325
fmgr_info(eq_function, &(peraggstate->equalfn));
1331-
peraggstate->sortOperator = any_ordering_op(inputType);
1326+
peraggstate->sortOperator = ordering_oper_opid(inputType);
13321327
peraggstate->sortstate = NULL;
13331328
}
13341329

‎src/backend/executor/nodeGroup.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* locate group boundaries.
1616
*
1717
* IDENTIFICATION
18-
* $Header: /cvsroot/pgsql/src/backend/executor/nodeGroup.c,v 1.49 2002/11/06 22:31:23 tgl Exp $
18+
* $Header: /cvsroot/pgsql/src/backend/executor/nodeGroup.c,v 1.50 2002/11/29 21:39:11 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -353,11 +353,7 @@ execTuplesMatchPrepare(TupleDesc tupdesc,
353353
Oid typid = tupdesc->attrs[att - 1]->atttypid;
354354
Oid eq_function;
355355

356-
eq_function = compatible_oper_funcid(makeList1(makeString("=")),
357-
typid, typid, true);
358-
if (!OidIsValid(eq_function))
359-
elog(ERROR, "Unable to identify an equality operator for type %s",
360-
format_type_be(typid));
356+
eq_function = equality_oper_funcid(typid);
361357
fmgr_info(eq_function, &eqfunctions[i]);
362358
}
363359

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

Lines changed: 36 additions & 2 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.131 2002/11/26 03:01:58 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.132 2002/11/29 21:39:11 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -17,6 +17,7 @@
1717

1818
#include <limits.h>
1919

20+
#include "catalog/pg_operator.h"
2021
#include "catalog/pg_type.h"
2122
#include "miscadmin.h"
2223
#include "nodes/makefuncs.h"
@@ -36,9 +37,11 @@
3637
#include "parser/analyze.h"
3738
#include "parser/parsetree.h"
3839
#include "parser/parse_expr.h"
40+
#include "parser/parse_oper.h"
3941
#include "rewrite/rewriteManip.h"
4042
#include "utils/lsyscache.h"
4143
#include "utils/selfuncs.h"
44+
#include "utils/syscache.h"
4245

4346

4447
/* Expression kind codes for preprocess_expression */
@@ -57,6 +60,7 @@ static Node *preprocess_expression(Query *parse, Node *expr, int kind);
5760
static void preprocess_qual_conditions(Query *parse, Node *jtnode);
5861
static Plan *inheritance_planner(Query *parse, List *inheritlist);
5962
static Plan *grouping_planner(Query *parse, double tuple_fraction);
63+
static bool hash_safe_grouping(Query *parse);
6064
static List *make_subplanTargetList(Query *parse, List *tlist,
6165
AttrNumber **groupColIdx);
6266
static Plan *make_groupsortplan(Query *parse,
@@ -1252,11 +1256,14 @@ grouping_planner(Query *parse, double tuple_fraction)
12521256
numGroups = (long) Min(dNumGroups, (double) LONG_MAX);
12531257

12541258
/*
1259+
* Check can't-do-it conditions, including whether the grouping
1260+
* operators are hashjoinable.
1261+
*
12551262
* Executor doesn't support hashed aggregation with DISTINCT
12561263
* aggregates. (Doing so would imply storing *all* the input
12571264
* values in the hash table, which seems like a certain loser.)
12581265
*/
1259-
if (!enable_hashagg)
1266+
if (!enable_hashagg || !hash_safe_grouping(parse))
12601267
use_hashed_grouping = false;
12611268
else if (parse->hasAggs &&
12621269
(contain_distinct_agg_clause((Node *) tlist) ||
@@ -1554,6 +1561,33 @@ grouping_planner(Query *parse, double tuple_fraction)
15541561
return result_plan;
15551562
}
15561563

1564+
/*
1565+
* hash_safe_grouping - are grouping operators hashable?
1566+
*
1567+
* We assume hashed aggregation will work if the datatype's equality operator
1568+
* is marked hashjoinable.
1569+
*/
1570+
static bool
1571+
hash_safe_grouping(Query *parse)
1572+
{
1573+
List *gl;
1574+
1575+
foreach(gl, parse->groupClause)
1576+
{
1577+
GroupClause *grpcl = (GroupClause *) lfirst(gl);
1578+
TargetEntry *tle = get_sortgroupclause_tle(grpcl, parse->targetList);
1579+
Operator optup;
1580+
bool oprcanhash;
1581+
1582+
optup = equality_oper(tle->resdom->restype, false);
1583+
oprcanhash = ((Form_pg_operator) GETSTRUCT(optup))->oprcanhash;
1584+
ReleaseSysCache(optup);
1585+
if (!oprcanhash)
1586+
return false;
1587+
}
1588+
return true;
1589+
}
1590+
15571591
/*---------------
15581592
* make_subplanTargetList
15591593
* Generate appropriate target list when grouping is required.

‎src/backend/parser/parse_clause.c

Lines changed: 5 additions & 6 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.99 2002/11/15 02:50:08 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.100 2002/11/29 21:39:11 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1128,8 +1128,7 @@ findTargetlistEntry(ParseState *pstate, Node *node, List *tlist, int clause)
11281128

11291129
/*
11301130
* transformGroupClause -
1131-
* transform a Group By clause
1132-
*
1131+
* transform a GROUP BY clause
11331132
*/
11341133
List *
11351134
transformGroupClause(ParseState *pstate, List *grouplist, List *targetlist)
@@ -1151,7 +1150,7 @@ transformGroupClause(ParseState *pstate, List *grouplist, List *targetlist)
11511150

11521151
grpcl->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
11531152

1154-
grpcl->sortop = any_ordering_op(tle->resdom->restype);
1153+
grpcl->sortop = ordering_oper_opid(tle->resdom->restype);
11551154

11561155
glist = lappend(glist, grpcl);
11571156
}
@@ -1331,7 +1330,7 @@ addAllTargetsToSortList(List *sortlist, List *targetlist)
13311330
* addTargetToSortList
13321331
* If the given targetlist entry isn't already in the ORDER BY list,
13331332
* add it to the end of the list, using the sortop with given name
1334-
* or any available sort operator if opname == NIL.
1333+
* or the default sort operator if opname == NIL.
13351334
*
13361335
* Returns the updated ORDER BY list.
13371336
*/
@@ -1352,7 +1351,7 @@ addTargetToSortList(TargetEntry *tle, List *sortlist, List *targetlist,
13521351
tle->resdom->restype,
13531352
false);
13541353
else
1355-
sortcl->sortop = any_ordering_op(tle->resdom->restype);
1354+
sortcl->sortop = ordering_oper_opid(tle->resdom->restype);
13561355

13571356
sortlist = lappend(sortlist, sortcl);
13581357
}

‎src/backend/parser/parse_oper.c

Lines changed: 105 additions & 33 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_oper.c,v 1.60 2002/09/18 21:35:22 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_oper.c,v 1.61 2002/11/29 21:39:11 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -130,22 +130,116 @@ LookupOperNameTypeNames(List *opername, TypeName *oprleft,
130130
return operoid;
131131
}
132132

133+
/*
134+
* equality_oper - identify a suitable equality operator for a datatype
135+
*
136+
* On failure, return NULL if noError, else report a standard error
137+
*/
138+
Operator
139+
equality_oper(Oid argtype, bool noError)
140+
{
141+
Operator optup;
133142

134-
/* Select an ordering operator for the given datatype */
135-
Oid
136-
any_ordering_op(Oid argtype)
143+
/*
144+
* Look for an "=" operator for the datatype. We require it to be
145+
* an exact or binary-compatible match, since most callers are not
146+
* prepared to cope with adding any run-time type coercion steps.
147+
*/
148+
optup = compatible_oper(makeList1(makeString("=")),
149+
argtype, argtype, true);
150+
if (optup != NULL)
151+
{
152+
/*
153+
* Only believe that it's equality if it's mergejoinable,
154+
* hashjoinable, or uses eqsel() as oprrest.
155+
*/
156+
Form_pg_operator pgopform = (Form_pg_operator) GETSTRUCT(optup);
157+
158+
if (OidIsValid(pgopform->oprlsortop) ||
159+
pgopform->oprcanhash ||
160+
pgopform->oprrest == F_EQSEL)
161+
return optup;
162+
163+
ReleaseSysCache(optup);
164+
}
165+
if (!noError)
166+
elog(ERROR, "Unable to identify an equality operator for type %s",
167+
format_type_be(argtype));
168+
return NULL;
169+
}
170+
171+
/*
172+
* ordering_oper - identify a suitable sorting operator ("<") for a datatype
173+
*
174+
* On failure, return NULL if noError, else report a standard error
175+
*/
176+
Operator
177+
ordering_oper(Oid argtype, bool noError)
137178
{
138-
Oid order_opid;
179+
Operator optup;
139180

140-
order_opid = compatible_oper_opid(makeList1(makeString("<")),
141-
argtype, argtype, true);
142-
if (!OidIsValid(order_opid))
143-
elog(ERROR, "Unable to identify an ordering operator '%s' for type '%s'"
181+
/*
182+
* Find the type's equality operator, and use its lsortop (it *must*
183+
* be mergejoinable). We use this definition because for sorting and
184+
* grouping purposes, it's important that the equality and ordering
185+
* operators are consistent.
186+
*/
187+
optup = equality_oper(argtype, noError);
188+
if (optup != NULL)
189+
{
190+
Oid lsortop = ((Form_pg_operator) GETSTRUCT(optup))->oprlsortop;
191+
192+
ReleaseSysCache(optup);
193+
194+
if (OidIsValid(lsortop))
195+
{
196+
optup = SearchSysCache(OPEROID,
197+
ObjectIdGetDatum(lsortop),
198+
0, 0, 0);
199+
if (optup != NULL)
200+
return optup;
201+
}
202+
}
203+
if (!noError)
204+
elog(ERROR, "Unable to identify an ordering operator for type %s"
144205
"\n\tUse an explicit ordering operator or modify the query",
145-
"<", format_type_be(argtype));
146-
return order_opid;
206+
format_type_be(argtype));
207+
return NULL;
208+
}
209+
210+
/*
211+
* equality_oper_funcid - convenience routine for oprfuncid(equality_oper())
212+
*/
213+
Oid
214+
equality_oper_funcid(Oid argtype)
215+
{
216+
Operator optup;
217+
Oid result;
218+
219+
optup = equality_oper(argtype, false);
220+
result = oprfuncid(optup);
221+
ReleaseSysCache(optup);
222+
return result;
223+
}
224+
225+
/*
226+
* ordering_oper_opid - convenience routine for oprid(ordering_oper())
227+
*
228+
* This was formerly called any_ordering_op()
229+
*/
230+
Oid
231+
ordering_oper_opid(Oid argtype)
232+
{
233+
Operator optup;
234+
Oid result;
235+
236+
optup = ordering_oper(argtype, false);
237+
result = oprid(optup);
238+
ReleaseSysCache(optup);
239+
return result;
147240
}
148241

242+
149243
/* given operator tuple, return the operator OID */
150244
Oid
151245
oprid(Operator op)
@@ -731,28 +825,6 @@ compatible_oper_opid(List *op, Oid arg1, Oid arg2, bool noError)
731825
return InvalidOid;
732826
}
733827

734-
/* compatible_oper_funcid() -- get OID of a binary operator's function
735-
*
736-
* This is a convenience routine that extracts only the function OID
737-
* from the result of compatible_oper(). InvalidOid is returned if the
738-
* lookup fails and noError is true.
739-
*/
740-
Oid
741-
compatible_oper_funcid(List *op, Oid arg1, Oid arg2, bool noError)
742-
{
743-
Operator optup;
744-
Oid result;
745-
746-
optup = compatible_oper(op, arg1, arg2, noError);
747-
if (optup != NULL)
748-
{
749-
result = oprfuncid(optup);
750-
ReleaseSysCache(optup);
751-
return result;
752-
}
753-
return InvalidOid;
754-
}
755-
756828

757829
/* right_oper() -- search for a unary right operator (operator on right)
758830
* Given operator name and type of arg, return oper struct.

0 commit comments

Comments
 (0)