]> granicus.if.org Git - postgresql/commitdiff
Remove assumptions that not-equals operators cannot be in any opclass.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 6 Jul 2011 18:53:16 +0000 (14:53 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 6 Jul 2011 18:53:42 +0000 (14:53 -0400)
get_op_btree_interpretation assumed this in order to save some duplication
of code, but it's not true in general anymore because we added <> support
to btree_gist.  (We still assume it for btree opclasses, though.)

Also, essentially the same logic was baked into predtest.c.  Get rid of
that duplication by generalizing get_op_btree_interpretation so that it
can be used by predtest.c.

Per bug report from Denis de Bernardy and investigation by Jeff Davis,
though I didn't use Jeff's patch exactly as-is.

Back-patch to 9.1; we do not support this usage before that.

src/backend/optimizer/util/predtest.c
src/backend/parser/parse_expr.c
src/backend/utils/cache/lsyscache.c
src/include/utils/lsyscache.h

index a7e83729b1b1bd161fab50b00847cd883ca33676..bbfbc7947b1d890a06fa5b9c09e2c65512561580 100644 (file)
@@ -1249,6 +1249,7 @@ list_member_strip(List *list, Expr *datum)
  * and in addition we use (6) to represent <>. <> is not a btree-indexable
  * operator, but we assume here that if an equality operator of a btree
  * opfamily has a negator operator, the negator behaves as <> for the opfamily.
+ * (This convention is also known to get_op_btree_interpretation().)
  *
  * The interpretation of:
  *
@@ -1285,7 +1286,7 @@ list_member_strip(List *list, Expr *datum)
 #define BTEQ BTEqualStrategyNumber
 #define BTGE BTGreaterEqualStrategyNumber
 #define BTGT BTGreaterStrategyNumber
-#define BTNE 6
+#define BTNE ROWCOMPARE_NE
 
 static const StrategyNumber BT_implic_table[6][6] = {
 /*
@@ -1556,18 +1557,12 @@ get_btree_test_op(Oid pred_op, Oid clause_op, bool refute_it)
        OprProofCacheKey key;
        OprProofCacheEntry *cache_entry;
        bool            cfound;
-       bool            pred_op_negated;
-       Oid                     pred_op_negator,
-                               clause_op_negator,
-                               test_op = InvalidOid;
-       Oid                     opfamily_id;
+       Oid                     test_op = InvalidOid;
        bool            found = false;
-       StrategyNumber pred_strategy,
-                               clause_strategy,
-                               test_strategy;
-       Oid                     clause_righttype;
-       CatCList   *catlist;
-       int                     i;
+       List       *pred_op_infos,
+                          *clause_op_infos;
+       ListCell   *lcp,
+                          *lcc;
 
        /*
         * Find or make a cache entry for this pair of operators.
@@ -1628,135 +1623,71 @@ get_btree_test_op(Oid pred_op, Oid clause_op, bool refute_it)
         * corresponding test operator.  This should work for any logically
         * consistent opfamilies.
         */
-       catlist = SearchSysCacheList1(AMOPOPID, ObjectIdGetDatum(pred_op));
+       clause_op_infos = get_op_btree_interpretation(clause_op);
+       if (clause_op_infos)
+               pred_op_infos = get_op_btree_interpretation(pred_op);
+       else                                                    /* no point in looking */
+               pred_op_infos = NIL;
 
-       /*
-        * If we couldn't find any opfamily containing the pred_op, perhaps it is
-        * a <> operator.  See if it has a negator that is in an opfamily.
-        */
-       pred_op_negated = false;
-       if (catlist->n_members == 0)
+       foreach(lcp, pred_op_infos)
        {
-               pred_op_negator = get_negator(pred_op);
-               if (OidIsValid(pred_op_negator))
-               {
-                       pred_op_negated = true;
-                       ReleaseSysCacheList(catlist);
-                       catlist = SearchSysCacheList1(AMOPOPID,
-                                                                                 ObjectIdGetDatum(pred_op_negator));
-               }
-       }
+               OpBtreeInterpretation *pred_op_info = lfirst(lcp);
+               Oid                     opfamily_id = pred_op_info->opfamily_id;
 
-       /* Also may need the clause_op's negator */
-       clause_op_negator = get_negator(clause_op);
+               foreach(lcc, clause_op_infos)
+               {
+                       OpBtreeInterpretation *clause_op_info = lfirst(lcc);
+                       StrategyNumber pred_strategy,
+                                               clause_strategy,
+                                               test_strategy;
 
-       /* Now search the opfamilies */
-       for (i = 0; i < catlist->n_members; i++)
-       {
-               HeapTuple       pred_tuple = &catlist->members[i]->tuple;
-               Form_pg_amop pred_form = (Form_pg_amop) GETSTRUCT(pred_tuple);
-               HeapTuple       clause_tuple;
+                       /* Must find them in same opfamily */
+                       if (opfamily_id != clause_op_info->opfamily_id)
+                               continue;
+                       /* Lefttypes should match */
+                       Assert(clause_op_info->oplefttype == pred_op_info->oplefttype);
 
-               /* Must be btree */
-               if (pred_form->amopmethod != BTREE_AM_OID)
-                       continue;
+                       pred_strategy = pred_op_info->strategy;
+                       clause_strategy = clause_op_info->strategy;
 
-               /* Get the predicate operator's btree strategy number */
-               opfamily_id = pred_form->amopfamily;
-               pred_strategy = (StrategyNumber) pred_form->amopstrategy;
-               Assert(pred_strategy >= 1 && pred_strategy <= 5);
+                       /*
+                        * Look up the "test" strategy number in the implication table
+                        */
+                       if (refute_it)
+                               test_strategy = BT_refute_table[clause_strategy - 1][pred_strategy - 1];
+                       else
+                               test_strategy = BT_implic_table[clause_strategy - 1][pred_strategy - 1];
 
-               if (pred_op_negated)
-               {
-                       /* Only consider negators that are = */
-                       if (pred_strategy != BTEqualStrategyNumber)
+                       if (test_strategy == 0)
+                       {
+                               /* Can't determine implication using this interpretation */
                                continue;
-                       pred_strategy = BTNE;
-               }
+                       }
 
-               /*
-                * From the same opfamily, find a strategy number for the clause_op,
-                * if possible
-                */
-               clause_tuple = SearchSysCache3(AMOPOPID,
-                                                                          ObjectIdGetDatum(clause_op),
-                                                                          CharGetDatum(AMOP_SEARCH),
-                                                                          ObjectIdGetDatum(opfamily_id));
-               if (HeapTupleIsValid(clause_tuple))
-               {
-                       Form_pg_amop clause_form = (Form_pg_amop) GETSTRUCT(clause_tuple);
-
-                       /* Get the restriction clause operator's strategy/datatype */
-                       clause_strategy = (StrategyNumber) clause_form->amopstrategy;
-                       Assert(clause_strategy >= 1 && clause_strategy <= 5);
-                       Assert(clause_form->amoplefttype == pred_form->amoplefttype);
-                       clause_righttype = clause_form->amoprighttype;
-                       ReleaseSysCache(clause_tuple);
-               }
-               else if (OidIsValid(clause_op_negator))
-               {
-                       clause_tuple = SearchSysCache3(AMOPOPID,
-                                                                                ObjectIdGetDatum(clause_op_negator),
-                                                                                  CharGetDatum(AMOP_SEARCH),
-                                                                                  ObjectIdGetDatum(opfamily_id));
-                       if (HeapTupleIsValid(clause_tuple))
+                       /*
+                        * See if opfamily has an operator for the test strategy and the
+                        * datatypes.
+                        */
+                       if (test_strategy == BTNE)
                        {
-                               Form_pg_amop clause_form = (Form_pg_amop) GETSTRUCT(clause_tuple);
-
-                               /* Get the restriction clause operator's strategy/datatype */
-                               clause_strategy = (StrategyNumber) clause_form->amopstrategy;
-                               Assert(clause_strategy >= 1 && clause_strategy <= 5);
-                               Assert(clause_form->amoplefttype == pred_form->amoplefttype);
-                               clause_righttype = clause_form->amoprighttype;
-                               ReleaseSysCache(clause_tuple);
-
-                               /* Only consider negators that are = */
-                               if (clause_strategy != BTEqualStrategyNumber)
-                                       continue;
-                               clause_strategy = BTNE;
+                               test_op = get_opfamily_member(opfamily_id,
+                                                                                         pred_op_info->oprighttype,
+                                                                                         clause_op_info->oprighttype,
+                                                                                         BTEqualStrategyNumber);
+                               if (OidIsValid(test_op))
+                                       test_op = get_negator(test_op);
                        }
                        else
-                               continue;
-               }
-               else
-                       continue;
-
-               /*
-                * Look up the "test" strategy number in the implication table
-                */
-               if (refute_it)
-                       test_strategy = BT_refute_table[clause_strategy - 1][pred_strategy - 1];
-               else
-                       test_strategy = BT_implic_table[clause_strategy - 1][pred_strategy - 1];
+                       {
+                               test_op = get_opfamily_member(opfamily_id,
+                                                                                         pred_op_info->oprighttype,
+                                                                                         clause_op_info->oprighttype,
+                                                                                         test_strategy);
+                       }
 
-               if (test_strategy == 0)
-               {
-                       /* Can't determine implication using this interpretation */
-                       continue;
-               }
+                       if (!OidIsValid(test_op))
+                               continue;
 
-               /*
-                * See if opfamily has an operator for the test strategy and the
-                * datatypes.
-                */
-               if (test_strategy == BTNE)
-               {
-                       test_op = get_opfamily_member(opfamily_id,
-                                                                                 pred_form->amoprighttype,
-                                                                                 clause_righttype,
-                                                                                 BTEqualStrategyNumber);
-                       if (OidIsValid(test_op))
-                               test_op = get_negator(test_op);
-               }
-               else
-               {
-                       test_op = get_opfamily_member(opfamily_id,
-                                                                                 pred_form->amoprighttype,
-                                                                                 clause_righttype,
-                                                                                 test_strategy);
-               }
-               if (OidIsValid(test_op))
-               {
                        /*
                         * Last check: test_op must be immutable.
                         *
@@ -1772,9 +1703,13 @@ get_btree_test_op(Oid pred_op, Oid clause_op, bool refute_it)
                                break;
                        }
                }
+
+               if (found)
+                       break;
        }
 
-       ReleaseSysCacheList(catlist);
+       list_free_deep(pred_op_infos);
+       list_free_deep(clause_op_infos);
 
        if (!found)
        {
index 08f0439e7edb270176e4b2c60cd92997ab745d69..65d03adc4942e16ecbf0dd057eaa5f90a534a4e1 100644 (file)
@@ -2170,8 +2170,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
        List       *opfamilies;
        ListCell   *l,
                           *r;
-       List      **opfamily_lists;
-       List      **opstrat_lists;
+       List      **opinfo_lists;
        Bitmapset  *strats;
        int                     nopers;
        int                     i;
@@ -2241,8 +2240,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
         * containing the operators, and see which interpretations (strategy
         * numbers) exist for each operator.
         */
-       opfamily_lists = (List **) palloc(nopers * sizeof(List *));
-       opstrat_lists = (List **) palloc(nopers * sizeof(List *));
+       opinfo_lists = (List **) palloc(nopers * sizeof(List *));
        strats = NULL;
        i = 0;
        foreach(l, opexprs)
@@ -2251,17 +2249,18 @@ make_row_comparison_op(ParseState *pstate, List *opname,
                Bitmapset  *this_strats;
                ListCell   *j;
 
-               get_op_btree_interpretation(opno,
-                                                                       &opfamily_lists[i], &opstrat_lists[i]);
+               opinfo_lists[i] = get_op_btree_interpretation(opno);
 
                /*
-                * convert strategy number list to a Bitmapset to make the
+                * convert strategy numbers into a Bitmapset to make the
                 * intersection calculation easy.
                 */
                this_strats = NULL;
-               foreach(j, opstrat_lists[i])
+               foreach(j, opinfo_lists[i])
                {
-                       this_strats = bms_add_member(this_strats, lfirst_int(j));
+                       OpBtreeInterpretation *opinfo = lfirst(j);
+
+                       this_strats = bms_add_member(this_strats, opinfo->strategy);
                }
                if (i == 0)
                        strats = this_strats;
@@ -2309,14 +2308,15 @@ make_row_comparison_op(ParseState *pstate, List *opname,
        for (i = 0; i < nopers; i++)
        {
                Oid                     opfamily = InvalidOid;
+               ListCell   *j;
 
-               forboth(l, opfamily_lists[i], r, opstrat_lists[i])
+               foreach(j, opinfo_lists[i])
                {
-                       int                     opstrat = lfirst_int(r);
+                       OpBtreeInterpretation *opinfo = lfirst(j);
 
-                       if (opstrat == rctype)
+                       if (opinfo->strategy == rctype)
                        {
-                               opfamily = lfirst_oid(l);
+                               opfamily = opinfo->opfamily_id;
                                break;
                        }
                }
index 28d18b0d32d9567a1e45313f79d49eb1431b0aaf..a59bb1bc1a0fdebfbc5711a8c04eae8db3fbd084 100644 (file)
@@ -622,52 +622,30 @@ get_op_hash_functions(Oid opno,
 /*
  * get_op_btree_interpretation
  *             Given an operator's OID, find out which btree opfamilies it belongs to,
- *             and what strategy number it has within each one.  The results are
- *             returned as an OID list and a parallel integer list.
+ *             and what properties it has within each one.  The results are returned
+ *             as a palloc'd list of OpBtreeInterpretation structs.
  *
  * In addition to the normal btree operators, we consider a <> operator to be
  * a "member" of an opfamily if its negator is an equality operator of the
  * opfamily.  ROWCOMPARE_NE is returned as the strategy number for this case.
  */
-void
-get_op_btree_interpretation(Oid opno, List **opfamilies, List **opstrats)
+List *
+get_op_btree_interpretation(Oid opno)
 {
+       List       *result = NIL;
+       OpBtreeInterpretation *thisresult;
        CatCList   *catlist;
-       bool            op_negated;
        int                     i;
 
-       *opfamilies = NIL;
-       *opstrats = NIL;
-
        /*
         * Find all the pg_amop entries containing the operator.
         */
        catlist = SearchSysCacheList1(AMOPOPID, ObjectIdGetDatum(opno));
 
-       /*
-        * If we can't find any opfamily containing the op, perhaps it is a <>
-        * operator.  See if it has a negator that is in an opfamily.
-        */
-       op_negated = false;
-       if (catlist->n_members == 0)
-       {
-               Oid                     op_negator = get_negator(opno);
-
-               if (OidIsValid(op_negator))
-               {
-                       op_negated = true;
-                       ReleaseSysCacheList(catlist);
-                       catlist = SearchSysCacheList1(AMOPOPID,
-                                                                                 ObjectIdGetDatum(op_negator));
-               }
-       }
-
-       /* Now search the opfamilies */
        for (i = 0; i < catlist->n_members; i++)
        {
                HeapTuple       op_tuple = &catlist->members[i]->tuple;
                Form_pg_amop op_form = (Form_pg_amop) GETSTRUCT(op_tuple);
-               Oid                     opfamily_id;
                StrategyNumber op_strategy;
 
                /* must be btree */
@@ -675,23 +653,66 @@ get_op_btree_interpretation(Oid opno, List **opfamilies, List **opstrats)
                        continue;
 
                /* Get the operator's btree strategy number */
-               opfamily_id = op_form->amopfamily;
                op_strategy = (StrategyNumber) op_form->amopstrategy;
                Assert(op_strategy >= 1 && op_strategy <= 5);
 
-               if (op_negated)
+               thisresult = (OpBtreeInterpretation *)
+                       palloc(sizeof(OpBtreeInterpretation));
+               thisresult->opfamily_id = op_form->amopfamily;
+               thisresult->strategy = op_strategy;
+               thisresult->oplefttype = op_form->amoplefttype;
+               thisresult->oprighttype = op_form->amoprighttype;
+               result = lappend(result, thisresult);
+       }
+
+       ReleaseSysCacheList(catlist);
+
+       /*
+        * If we didn't find any btree opfamily containing the operator, perhaps
+        * it is a <> operator.  See if it has a negator that is in an opfamily.
+        */
+       if (result == NIL)
+       {
+               Oid                     op_negator = get_negator(opno);
+
+               if (OidIsValid(op_negator))
                {
-                       /* Only consider negators that are = */
-                       if (op_strategy != BTEqualStrategyNumber)
-                               continue;
-                       op_strategy = ROWCOMPARE_NE;
-               }
+                       catlist = SearchSysCacheList1(AMOPOPID,
+                                                                                 ObjectIdGetDatum(op_negator));
 
-               *opfamilies = lappend_oid(*opfamilies, opfamily_id);
-               *opstrats = lappend_int(*opstrats, op_strategy);
+                       for (i = 0; i < catlist->n_members; i++)
+                       {
+                               HeapTuple       op_tuple = &catlist->members[i]->tuple;
+                               Form_pg_amop op_form = (Form_pg_amop) GETSTRUCT(op_tuple);
+                               StrategyNumber op_strategy;
+
+                               /* must be btree */
+                               if (op_form->amopmethod != BTREE_AM_OID)
+                                       continue;
+
+                               /* Get the operator's btree strategy number */
+                               op_strategy = (StrategyNumber) op_form->amopstrategy;
+                               Assert(op_strategy >= 1 && op_strategy <= 5);
+
+                               /* Only consider negators that are = */
+                               if (op_strategy != BTEqualStrategyNumber)
+                                       continue;
+
+                               /* OK, report it with "strategy" ROWCOMPARE_NE */
+                               thisresult = (OpBtreeInterpretation *)
+                                       palloc(sizeof(OpBtreeInterpretation));
+                               thisresult->opfamily_id = op_form->amopfamily;
+                               thisresult->strategy = ROWCOMPARE_NE;
+                               thisresult->oplefttype = op_form->amoplefttype;
+                               thisresult->oprighttype = op_form->amoprighttype;
+                               result = lappend(result, thisresult);
+                       }
+
+                       ReleaseSysCacheList(catlist);
+               }
        }
 
-       ReleaseSysCacheList(catlist);
+       return result;
 }
 
 /*
index 0a419dcf65b4b3ed5ec9905aa06c567602cc66e1..f4490adc318921633aec34b06b82937d5237eb81 100644 (file)
 #include "access/htup.h"
 #include "nodes/pg_list.h"
 
+/* Result list element for get_op_btree_interpretation */
+typedef struct OpBtreeInterpretation
+{
+       Oid                     opfamily_id;            /* btree opfamily containing operator */
+       int                     strategy;                       /* its strategy number */
+       Oid                     oplefttype;                     /* declared left input datatype */
+       Oid                     oprighttype;            /* declared right input datatype */
+} OpBtreeInterpretation;
+
 /* I/O function selector for get_type_io_data */
 typedef enum IOFuncSelector
 {
@@ -50,8 +59,7 @@ extern bool get_compatible_hash_operators(Oid opno,
                                                          Oid *lhs_opno, Oid *rhs_opno);
 extern bool get_op_hash_functions(Oid opno,
                                          RegProcedure *lhs_procno, RegProcedure *rhs_procno);
-extern void get_op_btree_interpretation(Oid opno,
-                                                       List **opfamilies, List **opstrats);
+extern List *get_op_btree_interpretation(Oid opno);
 extern bool equality_ops_are_compatible(Oid opno1, Oid opno2);
 extern Oid get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype,
                                  int16 procnum);