]> granicus.if.org Git - postgresql/commitdiff
Tighten selection of equality and ordering operators for grouping
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 29 Nov 2002 21:39:12 +0000 (21:39 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 29 Nov 2002 21:39:12 +0000 (21:39 +0000)
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.

src/backend/commands/analyze.c
src/backend/executor/nodeAgg.c
src/backend/executor/nodeGroup.c
src/backend/optimizer/plan/planner.c
src/backend/parser/parse_clause.c
src/backend/parser/parse_oper.c
src/backend/utils/adt/ri_triggers.c
src/include/parser/parse_oper.h

index e15320225da700fddb3e2b6a27a746cfd7f63b5d..2af7a3cd7c322d242225229b0bc2e871f9da4542 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.50 2002/11/13 00:39:46 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/commands/analyze.c,v 1.51 2002/11/29 21:39:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -402,10 +402,7 @@ examine_attribute(Relation onerel, int attnum)
                return NULL;
 
        /* If column has no "=" operator, we can't do much of anything */
-       func_operator = compatible_oper(makeList1(makeString("=")),
-                                                                       attr->atttypid,
-                                                                       attr->atttypid,
-                                                                       true);
+       func_operator = equality_oper(attr->atttypid, true);
        if (func_operator != NULL)
        {
                oprrest = ((Form_pg_operator) GETSTRUCT(func_operator))->oprrest;
@@ -443,10 +440,7 @@ examine_attribute(Relation onerel, int attnum)
                stats->attr->attstattarget = default_statistics_target;
 
        /* Is there a "<" operator with suitable semantics? */
-       func_operator = compatible_oper(makeList1(makeString("<")),
-                                                                       attr->atttypid,
-                                                                       attr->atttypid,
-                                                                       true);
+       func_operator = ordering_oper(attr->atttypid, true);
        if (func_operator != NULL)
        {
                oprrest = ((Form_pg_operator) GETSTRUCT(func_operator))->oprrest;
index 0216f8ebde7fe83f122af02ff1682614204a5630..82e9f48e1e0ef069ab78510244b7695bd1f8236b 100644 (file)
@@ -45,7 +45,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v 1.96 2002/11/19 23:21:57 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v 1.97 2002/11/29 21:39:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1321,14 +1321,9 @@ ExecInitAgg(Agg *node, EState *estate, Plan *parent)
                                                        &peraggstate->inputtypeLen,
                                                        &peraggstate->inputtypeByVal);
 
-                       eq_function = compatible_oper_funcid(makeList1(makeString("=")),
-                                                                                                inputType, inputType,
-                                                                                                true);
-                       if (!OidIsValid(eq_function))
-                               elog(ERROR, "Unable to identify an equality operator for type %s",
-                                        format_type_be(inputType));
+                       eq_function = equality_oper_funcid(inputType);
                        fmgr_info(eq_function, &(peraggstate->equalfn));
-                       peraggstate->sortOperator = any_ordering_op(inputType);
+                       peraggstate->sortOperator = ordering_oper_opid(inputType);
                        peraggstate->sortstate = NULL;
                }
 
index 3ea0e44d286b2598cee7813c22bc98d3e3bbffad..d41bcbb6fcc4091c02c41c0814f576564afc2c3f 100644 (file)
@@ -15,7 +15,7 @@
  *       locate group boundaries.
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/executor/nodeGroup.c,v 1.49 2002/11/06 22:31:23 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/executor/nodeGroup.c,v 1.50 2002/11/29 21:39:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -353,11 +353,7 @@ execTuplesMatchPrepare(TupleDesc tupdesc,
                Oid                     typid = tupdesc->attrs[att - 1]->atttypid;
                Oid                     eq_function;
 
-               eq_function = compatible_oper_funcid(makeList1(makeString("=")),
-                                                                                        typid, typid, true);
-               if (!OidIsValid(eq_function))
-                       elog(ERROR, "Unable to identify an equality operator for type %s",
-                                format_type_be(typid));
+               eq_function = equality_oper_funcid(typid);
                fmgr_info(eq_function, &eqfunctions[i]);
        }
 
index 2c1081f2677ac7ddefaf9c24cbc63316c708dc76..0e5afccae3adede688a1ca71e17fab10c940a69d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.131 2002/11/26 03:01:58 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.132 2002/11/29 21:39:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,6 +17,7 @@
 
 #include <limits.h>
 
+#include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "parser/analyze.h"
 #include "parser/parsetree.h"
 #include "parser/parse_expr.h"
+#include "parser/parse_oper.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/lsyscache.h"
 #include "utils/selfuncs.h"
+#include "utils/syscache.h"
 
 
 /* Expression kind codes for preprocess_expression */
@@ -57,6 +60,7 @@ static Node *preprocess_expression(Query *parse, Node *expr, int kind);
 static void preprocess_qual_conditions(Query *parse, Node *jtnode);
 static Plan *inheritance_planner(Query *parse, List *inheritlist);
 static Plan *grouping_planner(Query *parse, double tuple_fraction);
+static bool hash_safe_grouping(Query *parse);
 static List *make_subplanTargetList(Query *parse, List *tlist,
                                           AttrNumber **groupColIdx);
 static Plan *make_groupsortplan(Query *parse,
@@ -1252,11 +1256,14 @@ grouping_planner(Query *parse, double tuple_fraction)
                        numGroups = (long) Min(dNumGroups, (double) LONG_MAX);
 
                        /*
+                        * Check can't-do-it conditions, including whether the grouping
+                        * operators are hashjoinable.
+                        *
                         * Executor doesn't support hashed aggregation with DISTINCT
                         * aggregates.  (Doing so would imply storing *all* the input
                         * values in the hash table, which seems like a certain loser.)
                         */
-                       if (!enable_hashagg)
+                       if (!enable_hashagg || !hash_safe_grouping(parse))
                                use_hashed_grouping = false;
                        else if (parse->hasAggs &&
                                         (contain_distinct_agg_clause((Node *) tlist) ||
@@ -1554,6 +1561,33 @@ grouping_planner(Query *parse, double tuple_fraction)
        return result_plan;
 }
 
+/*
+ * hash_safe_grouping - are grouping operators hashable?
+ *
+ * We assume hashed aggregation will work if the datatype's equality operator
+ * is marked hashjoinable.
+ */
+static bool
+hash_safe_grouping(Query *parse)
+{
+       List       *gl;
+
+       foreach(gl, parse->groupClause)
+       {
+               GroupClause *grpcl = (GroupClause *) lfirst(gl);
+               TargetEntry *tle = get_sortgroupclause_tle(grpcl, parse->targetList);
+               Operator        optup;
+               bool            oprcanhash;
+
+               optup = equality_oper(tle->resdom->restype, false);
+               oprcanhash = ((Form_pg_operator) GETSTRUCT(optup))->oprcanhash;
+               ReleaseSysCache(optup);
+               if (!oprcanhash)
+                       return false;
+       }
+       return true;
+}
+
 /*---------------
  * make_subplanTargetList
  *       Generate appropriate target list when grouping is required.
index d9638753746f7db869102ea2f6ea2731d0dcf5ed..ca398b4e3b31d4f50b4db0cd87e5ed409c7d19e0 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.99 2002/11/15 02:50:08 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.100 2002/11/29 21:39:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1128,8 +1128,7 @@ findTargetlistEntry(ParseState *pstate, Node *node, List *tlist, int clause)
 
 /*
  * transformGroupClause -
- *       transform a Group By clause
- *
+ *       transform a GROUP BY clause
  */
 List *
 transformGroupClause(ParseState *pstate, List *grouplist, List *targetlist)
@@ -1151,7 +1150,7 @@ transformGroupClause(ParseState *pstate, List *grouplist, List *targetlist)
 
                        grpcl->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
 
-                       grpcl->sortop = any_ordering_op(tle->resdom->restype);
+                       grpcl->sortop = ordering_oper_opid(tle->resdom->restype);
 
                        glist = lappend(glist, grpcl);
                }
@@ -1331,7 +1330,7 @@ addAllTargetsToSortList(List *sortlist, List *targetlist)
  * addTargetToSortList
  *             If the given targetlist entry isn't already in the ORDER BY list,
  *             add it to the end of the list, using the sortop with given name
- *             or any available sort operator if opname == NIL.
+ *             or the default sort operator if opname == NIL.
  *
  * Returns the updated ORDER BY list.
  */
@@ -1352,7 +1351,7 @@ addTargetToSortList(TargetEntry *tle, List *sortlist, List *targetlist,
                                                                                                  tle->resdom->restype,
                                                                                                  false);
                else
-                       sortcl->sortop = any_ordering_op(tle->resdom->restype);
+                       sortcl->sortop = ordering_oper_opid(tle->resdom->restype);
 
                sortlist = lappend(sortlist, sortcl);
        }
index 776acc78bfae1729354e14cf15e8a75454f5f3b6..eeb8f6aa8bb71a41c154e9c495854813d72b1708 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/parser/parse_oper.c,v 1.60 2002/09/18 21:35:22 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/parser/parse_oper.c,v 1.61 2002/11/29 21:39:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -130,22 +130,116 @@ LookupOperNameTypeNames(List *opername, TypeName *oprleft,
        return operoid;
 }
 
+/*
+ * equality_oper - identify a suitable equality operator for a datatype
+ *
+ * On failure, return NULL if noError, else report a standard error
+ */
+Operator
+equality_oper(Oid argtype, bool noError)
+{
+       Operator        optup;
 
-/* Select an ordering operator for the given datatype */
-Oid
-any_ordering_op(Oid argtype)
+       /*
+        * Look for an "=" operator for the datatype.  We require it to be
+        * an exact or binary-compatible match, since most callers are not
+        * prepared to cope with adding any run-time type coercion steps.
+        */
+       optup = compatible_oper(makeList1(makeString("=")),
+                                                       argtype, argtype, true);
+       if (optup != NULL)
+       {
+               /*
+                * Only believe that it's equality if it's mergejoinable,
+                * hashjoinable, or uses eqsel() as oprrest.
+                */
+               Form_pg_operator pgopform = (Form_pg_operator) GETSTRUCT(optup);
+
+               if (OidIsValid(pgopform->oprlsortop) ||
+                       pgopform->oprcanhash ||
+                       pgopform->oprrest == F_EQSEL)
+                       return optup;
+
+               ReleaseSysCache(optup);
+       }
+       if (!noError)
+               elog(ERROR, "Unable to identify an equality operator for type %s",
+                        format_type_be(argtype));
+       return NULL;
+}
+
+/*
+ * ordering_oper - identify a suitable sorting operator ("<") for a datatype
+ *
+ * On failure, return NULL if noError, else report a standard error
+ */
+Operator
+ordering_oper(Oid argtype, bool noError)
 {
-       Oid                     order_opid;
+       Operator        optup;
 
-       order_opid = compatible_oper_opid(makeList1(makeString("<")),
-                                                                         argtype, argtype, true);
-       if (!OidIsValid(order_opid))
-               elog(ERROR, "Unable to identify an ordering operator '%s' for type '%s'"
+       /*
+        * Find the type's equality operator, and use its lsortop (it *must*
+        * be mergejoinable).  We use this definition because for sorting and
+        * grouping purposes, it's important that the equality and ordering
+        * operators are consistent.
+        */
+       optup = equality_oper(argtype, noError);
+       if (optup != NULL)
+       {
+               Oid             lsortop = ((Form_pg_operator) GETSTRUCT(optup))->oprlsortop;
+
+               ReleaseSysCache(optup);
+
+               if (OidIsValid(lsortop))
+               {
+                       optup = SearchSysCache(OPEROID,
+                                                                  ObjectIdGetDatum(lsortop),
+                                                                  0, 0, 0);
+                       if (optup != NULL)
+                               return optup;
+               }
+       }
+       if (!noError)
+               elog(ERROR, "Unable to identify an ordering operator for type %s"
                         "\n\tUse an explicit ordering operator or modify the query",
-                        "<", format_type_be(argtype));
-       return order_opid;
+                        format_type_be(argtype));
+       return NULL;
+}
+
+/*
+ * equality_oper_funcid - convenience routine for oprfuncid(equality_oper())
+ */
+Oid
+equality_oper_funcid(Oid argtype)
+{
+       Operator        optup;
+       Oid                     result;
+
+       optup = equality_oper(argtype, false);
+       result = oprfuncid(optup);
+       ReleaseSysCache(optup);
+       return result;
+}
+
+/*
+ * ordering_oper_opid - convenience routine for oprid(ordering_oper())
+ *
+ * This was formerly called any_ordering_op()
+ */
+Oid
+ordering_oper_opid(Oid argtype)
+{
+       Operator        optup;
+       Oid                     result;
+
+       optup = ordering_oper(argtype, false);
+       result = oprid(optup);
+       ReleaseSysCache(optup);
+       return result;
 }
 
+
 /* given operator tuple, return the operator OID */
 Oid
 oprid(Operator op)
@@ -731,28 +825,6 @@ compatible_oper_opid(List *op, Oid arg1, Oid arg2, bool noError)
        return InvalidOid;
 }
 
-/* compatible_oper_funcid() -- get OID of a binary operator's function
- *
- * This is a convenience routine that extracts only the function OID
- * from the result of compatible_oper().  InvalidOid is returned if the
- * lookup fails and noError is true.
- */
-Oid
-compatible_oper_funcid(List *op, Oid arg1, Oid arg2, bool noError)
-{
-       Operator        optup;
-       Oid                     result;
-
-       optup = compatible_oper(op, arg1, arg2, noError);
-       if (optup != NULL)
-       {
-               result = oprfuncid(optup);
-               ReleaseSysCache(optup);
-               return result;
-       }
-       return InvalidOid;
-}
-
 
 /* right_oper() -- search for a unary right operator (operator on right)
  * Given operator name and type of arg, return oper struct.
index 76cc1bdb54969a31a49c30ffbf4f024a367c1ad0..4b9cbfbaa30e28053a060a9b13440a9955b82c0a 100644 (file)
@@ -17,7 +17,7 @@
  *
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  *
- * $Header: /cvsroot/pgsql/src/backend/utils/adt/ri_triggers.c,v 1.43 2002/10/03 21:06:23 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/adt/ri_triggers.c,v 1.44 2002/11/29 21:39:11 tgl Exp $
  *
  * ----------
  */
@@ -3669,12 +3669,7 @@ ri_AttributesEqual(Oid typeid, Datum oldvalue, Datum newvalue)
                Oid                     opr_proc;
                FmgrInfo        finfo;
 
-               opr_proc = compatible_oper_funcid(makeList1(makeString("=")),
-                                                                                 typeid, typeid, true);
-               if (!OidIsValid(opr_proc))
-                       elog(ERROR,
-                       "ri_AttributesEqual(): cannot find '=' operator for type %u",
-                                typeid);
+               opr_proc = equality_oper_funcid(typeid);
 
                /*
                 * Since fmgr_info could fail, call it *before* creating the
index 8369689cdcf728b77b72a1abcb508ccbf381827b..398bad58cd25375e453218760e1036ac54f1af0a 100644 (file)
@@ -1,13 +1,13 @@
 /*-------------------------------------------------------------------------
  *
  * parse_oper.h
- *
+ *             handle operator things for parser
  *
  *
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parse_oper.h,v 1.22 2002/09/04 20:31:45 momjian Exp $
+ * $Id: parse_oper.h,v 1.23 2002/11/29 21:39:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,13 +36,14 @@ extern Operator compatible_oper(List *op, Oid arg1, Oid arg2, bool noError);
 
 /* currently no need for compatible_left_oper/compatible_right_oper */
 
-/* Convenience routines that call compatible_oper() and return either */
-/* the operator OID or the underlying function OID, or InvalidOid if fail */
-extern Oid     compatible_oper_opid(List *op, Oid arg1, Oid arg2, bool noError);
-extern Oid     compatible_oper_funcid(List *op, Oid arg1, Oid arg2, bool noError);
+/* Routines for identifying "=" and "<" operators for a type */
+extern Operator equality_oper(Oid argtype, bool noError);
+extern Operator ordering_oper(Oid argtype, bool noError);
 
-/* Convenience routine that packages a specific call on compatible_oper */
-extern Oid     any_ordering_op(Oid argtype);
+/* Convenience routines for common calls on the above */
+extern Oid     compatible_oper_opid(List *op, Oid arg1, Oid arg2, bool noError);
+extern Oid     equality_oper_funcid(Oid argtype);
+extern Oid     ordering_oper_opid(Oid argtype);
 
 /* Extract operator OID or underlying-function OID from an Operator tuple */
 extern Oid     oprid(Operator op);