Re: No = operator for opfamily 426 - Mailing list pgsql-bugs

I wrote:
> Manuel Rigger <rigger.manuel@gmail.com> writes:
>> Consider the following statements:

>> CREATE TABLE t0(c0 TEXT);
>> CREATE INDEX i0 ON t0(c0 bpchar_ops);
>> SELECT * FROM t0 WHERE t0.c0 LIKE ''; -- ERROR:  no = operator for opfamily 426

> Really what the error is showing is that like_support.c is being too
> aggressive by assuming that it'll necessarily find a matching opfamily
> member.  It should probably just silently fail if it can't construct
> the opclause it wants.

I pushed a stopgap fix that just does that, but I think really what we
ought to do about this is decouple like_support.c as far as possible
from the index opclass.  The notion that we choose the target operators
based on the opclass is really backwards now that I think about it.
The operators that represent the potentially indexscannable conditions
are determined by the LIKE/regex operator, and what we should do is
just ask whether the opclass can support them.

The "pattern" opclasses put a crimp in this position, but those can
now be seen to be legacy things not a model that future code is likely
to follow.  So I present the attached proposed patch that does things
this way.  The only short-term advantage is that it can handle applying
an exact-match LIKE to a hash opclass:

regression=# create table t (f1 text);
CREATE TABLE
regression=# create index on t using hash(f1);
CREATE INDEX
regression=# explain select * from t where f1 like 'foo';
                              QUERY PLAN
-----------------------------------------------------------------------
 Bitmap Heap Scan on t  (cost=4.05..14.20 rows=7 width=32)
   Filter: (f1 ~~ 'foo'::text)
   ->  Bitmap Index Scan on t_f1_idx  (cost=0.00..4.05 rows=7 width=0)
         Index Cond: (f1 = 'foo'::text)
(4 rows)

which isn't much of a gain, admittedly.  But now this code won't need
revision when we start to think about new index AMs with new opclasses
that happen to implement btree-ish semantics.

            regards, tom lane

diff --git a/src/backend/utils/adt/like_support.c b/src/backend/utils/adt/like_support.c
index 77cc378..e785435 100644
--- a/src/backend/utils/adt/like_support.c
+++ b/src/backend/utils/adt/like_support.c
@@ -39,6 +39,7 @@
 #include "access/htup_details.h"
 #include "access/stratnum.h"
 #include "catalog/pg_collation.h"
+#include "catalog/pg_operator.h"
 #include "catalog/pg_opfamily.h"
 #include "catalog/pg_statistic.h"
 #include "catalog/pg_type.h"
@@ -240,7 +241,10 @@ match_pattern_prefix(Node *leftop,
     Pattern_Prefix_Status pstatus;
     Oid            ldatatype;
     Oid            rdatatype;
-    Oid            oproid;
+    Oid            eqopr;
+    Oid            ltopr;
+    Oid            geopr;
+    bool        collation_aware;
     Expr       *expr;
     FmgrInfo    ltproc;
     Const       *greaterstr;
@@ -284,62 +288,89 @@ match_pattern_prefix(Node *leftop,
         return NIL;

     /*
-     * Must also check that index's opfamily supports the operators we will
-     * want to apply.  (A hash index, for example, will not support ">=".)
-     * Currently, only btree and spgist support the operators we need.
-     *
-     * Note: actually, in the Pattern_Prefix_Exact case, we only need "=" so a
-     * hash index would work.  Currently it doesn't seem worth checking for
-     * that, however.
-     *
-     * We insist on the opfamily being one of the specific ones we expect,
-     * else we'd do the wrong thing if someone were to make a reverse-sort
-     * opfamily with the same operators.
-     *
-     * The non-pattern opclasses will not sort the way we need in most non-C
-     * locales.  We can use such an index anyway for an exact match (simple
-     * equality), but not for prefix-match cases.  Note that here we are
-     * looking at the index's collation, not the expression's collation --
-     * this test is *not* dependent on the LIKE/regex operator's collation.
-     *
-     * While we're at it, identify the type the comparison constant(s) should
-     * have, based on the opfamily.
+     * Identify the operators we want to use, based on the type of the
+     * left-hand argument.  Usually these are just the type's regular
+     * comparison operators, but if we are considering one of the semi-legacy
+     * "pattern" opclasses, use the "pattern" operators instead.  Those are
+     * not collation-sensitive but always use C collation, as we want.  The
+     * selected operators also determine the needed type of the prefix
+     * constant.
      */
-    switch (opfamily)
+    ldatatype = exprType(leftop);
+    switch (ldatatype)
     {
-        case TEXT_BTREE_FAM_OID:
-            if (!(pstatus == Pattern_Prefix_Exact ||
-                  lc_collate_is_c(indexcollation)))
-                return NIL;
+        case TEXTOID:
+            if (opfamily == TEXT_PATTERN_BTREE_FAM_OID ||
+                opfamily == TEXT_SPGIST_FAM_OID)
+            {
+                eqopr = TextEqualOperator;
+                ltopr = TextPatternLessOperator;
+                geopr = TextPatternGreaterEqualOperator;
+                collation_aware = false;
+            }
+            else
+            {
+                eqopr = TextEqualOperator;
+                ltopr = TextLessOperator;
+                geopr = TextGreaterEqualOperator;
+                collation_aware = true;
+            }
             rdatatype = TEXTOID;
             break;
+        case NAMEOID:

-        case TEXT_PATTERN_BTREE_FAM_OID:
-        case TEXT_SPGIST_FAM_OID:
+            /*
+             * Note that here, we need the RHS type to be text, so that the
+             * comparison value isn't improperly truncated to NAMEDATALEN.
+             */
+            eqopr = NameEqualTextOperator;
+            ltopr = NameLessTextOperator;
+            geopr = NameGreaterEqualTextOperator;
+            collation_aware = true;
             rdatatype = TEXTOID;
             break;
-
-        case BPCHAR_BTREE_FAM_OID:
-            if (!(pstatus == Pattern_Prefix_Exact ||
-                  lc_collate_is_c(indexcollation)))
-                return NIL;
-            rdatatype = BPCHAROID;
-            break;
-
-        case BPCHAR_PATTERN_BTREE_FAM_OID:
+        case BPCHAROID:
+            if (opfamily == BPCHAR_PATTERN_BTREE_FAM_OID)
+            {
+                eqopr = BpcharEqualOperator;
+                ltopr = BpcharPatternLessOperator;
+                geopr = BpcharPatternGreaterEqualOperator;
+                collation_aware = false;
+            }
+            else
+            {
+                eqopr = BpcharEqualOperator;
+                ltopr = BpcharLessOperator;
+                geopr = BpcharGreaterEqualOperator;
+                collation_aware = true;
+            }
             rdatatype = BPCHAROID;
             break;
-
-        case BYTEA_BTREE_FAM_OID:
+        case BYTEAOID:
+            eqopr = ByteaEqualOperator;
+            ltopr = ByteaLessOperator;
+            geopr = ByteaGreaterEqualOperator;
+            collation_aware = false;
             rdatatype = BYTEAOID;
             break;
-
         default:
+            /* Can't get here unless we're attached to the wrong operator */
             return NIL;
     }

-    /* OK, prepare to create the indexqual(s) */
-    ldatatype = exprType(leftop);
+    /*
+     * If necessary, verify that the index's collation behavior is compatible.
+     * For an exact-match case, we don't have to be picky.  Otherwise, insist
+     * that the index collation be "C".  Note that here we are looking at the
+     * index's collation, not the expression's collation -- this test is *not*
+     * dependent on the LIKE/regex operator's collation.
+     */
+    if (collation_aware)
+    {
+        if (!(pstatus == Pattern_Prefix_Exact ||
+              lc_collate_is_c(indexcollation)))
+            return NIL;
+    }

     /*
      * If necessary, coerce the prefix constant to the right type.  The given
@@ -358,16 +389,17 @@ match_pattern_prefix(Node *leftop,
     /*
      * If we found an exact-match pattern, generate an "=" indexqual.
      *
-     * (Despite the checks above, we might fail to find a suitable operator in
-     * some cases with binary-compatible opclasses.  Just punt if so.)
+     * Here and below, check to see whether the desired operator is actually
+     * supported by the index opclass, and fail quietly if not.  This allows
+     * us to not be concerned with specific opclasses (except for the legacy
+     * "pattern" cases); any index that correctly implements the operators
+     * will work.
      */
     if (pstatus == Pattern_Prefix_Exact)
     {
-        oproid = get_opfamily_member(opfamily, ldatatype, rdatatype,
-                                     BTEqualStrategyNumber);
-        if (oproid == InvalidOid)
+        if (!op_in_opfamily(eqopr, opfamily))
             return NIL;
-        expr = make_opclause(oproid, BOOLOID, false,
+        expr = make_opclause(eqopr, BOOLOID, false,
                              (Expr *) leftop, (Expr *) prefix,
                              InvalidOid, indexcollation);
         result = list_make1(expr);
@@ -379,11 +411,9 @@ match_pattern_prefix(Node *leftop,
      *
      * We can always say "x >= prefix".
      */
-    oproid = get_opfamily_member(opfamily, ldatatype, rdatatype,
-                                 BTGreaterEqualStrategyNumber);
-    if (oproid == InvalidOid)
+    if (!op_in_opfamily(geopr, opfamily))
         return NIL;
-    expr = make_opclause(oproid, BOOLOID, false,
+    expr = make_opclause(geopr, BOOLOID, false,
                          (Expr *) leftop, (Expr *) prefix,
                          InvalidOid, indexcollation);
     result = list_make1(expr);
@@ -396,15 +426,13 @@ match_pattern_prefix(Node *leftop,
      * using a C-locale index collation.
      *-------
      */
-    oproid = get_opfamily_member(opfamily, ldatatype, rdatatype,
-                                 BTLessStrategyNumber);
-    if (oproid == InvalidOid)
+    if (!op_in_opfamily(ltopr, opfamily))
         return result;
-    fmgr_info(get_opcode(oproid), <proc);
+    fmgr_info(get_opcode(ltopr), <proc);
     greaterstr = make_greater_string(prefix, <proc, indexcollation);
     if (greaterstr)
     {
-        expr = make_opclause(oproid, BOOLOID, false,
+        expr = make_opclause(ltopr, BOOLOID, false,
                              (Expr *) leftop, (Expr *) greaterstr,
                              InvalidOid, indexcollation);
         result = lappend(result, expr);
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index fa7dc96..ba9c09d 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -107,12 +107,12 @@
   oprcode => 'starts_with', oprrest => 'prefixsel',
   oprjoin => 'prefixjoinsel' },

-{ oid => '254', descr => 'equal',
+{ oid => '254', oid_symbol => 'NameEqualTextOperator',  descr => 'equal',
   oprname => '=', oprcanmerge => 't', oprcanhash => 't', oprleft => 'name',
   oprright => 'text', oprresult => 'bool', oprcom => '=(text,name)',
   oprnegate => '<>(name,text)', oprcode => 'nameeqtext', oprrest => 'eqsel',
   oprjoin => 'eqjoinsel' },
-{ oid => '255', descr => 'less than',
+{ oid => '255', oid_symbol => 'NameLessTextOperator', descr => 'less than',
   oprname => '<', oprleft => 'name', oprright => 'text', oprresult => 'bool',
   oprcom => '>(text,name)', oprnegate => '>=(name,text)',
   oprcode => 'namelttext', oprrest => 'scalarltsel',
@@ -122,7 +122,7 @@
   oprcom => '>=(text,name)', oprnegate => '>(name,text)',
   oprcode => 'nameletext', oprrest => 'scalarlesel',
   oprjoin => 'scalarlejoinsel' },
-{ oid => '257', descr => 'greater than or equal',
+{ oid => '257', oid_symbol => 'NameGreaterEqualTextOperator', descr => 'greater than or equal',
   oprname => '>=', oprleft => 'name', oprright => 'text', oprresult => 'bool',
   oprcom => '<=(text,name)', oprnegate => '<(name,text)',
   oprcode => 'namegetext', oprrest => 'scalargesel',
@@ -789,7 +789,7 @@
   oprname => '>=', oprleft => 'name', oprright => 'name', oprresult => 'bool',
   oprcom => '<=(name,name)', oprnegate => '<(name,name)', oprcode => 'namege',
   oprrest => 'scalargesel', oprjoin => 'scalargejoinsel' },
-{ oid => '664', descr => 'less than',
+{ oid => '664', oid_symbol => 'TextLessOperator', descr => 'less than',
   oprname => '<', oprleft => 'text', oprright => 'text', oprresult => 'bool',
   oprcom => '>(text,text)', oprnegate => '>=(text,text)', oprcode => 'text_lt',
   oprrest => 'scalarltsel', oprjoin => 'scalarltjoinsel' },
@@ -801,7 +801,7 @@
   oprname => '>', oprleft => 'text', oprright => 'text', oprresult => 'bool',
   oprcom => '<(text,text)', oprnegate => '<=(text,text)', oprcode => 'text_gt',
   oprrest => 'scalargtsel', oprjoin => 'scalargtjoinsel' },
-{ oid => '667', descr => 'greater than or equal',
+{ oid => '667', oid_symbol => 'TextGreaterEqualOperator', descr => 'greater than or equal',
   oprname => '>=', oprleft => 'text', oprright => 'text', oprresult => 'bool',
   oprcom => '<=(text,text)', oprnegate => '<(text,text)', oprcode => 'text_ge',
   oprrest => 'scalargesel', oprjoin => 'scalargejoinsel' },
@@ -1160,7 +1160,7 @@
   oprname => '@@', oprkind => 'l', oprleft => '0', oprright => 'polygon',
   oprresult => 'point', oprcode => 'poly_center' },

-{ oid => '1054', descr => 'equal',
+{ oid => '1054', oid_symbol => 'BpcharEqualOperator', descr => 'equal',
   oprname => '=', oprcanmerge => 't', oprcanhash => 't', oprleft => 'bpchar',
   oprright => 'bpchar', oprresult => 'bool', oprcom => '=(bpchar,bpchar)',
   oprnegate => '<>(bpchar,bpchar)', oprcode => 'bpchareq', oprrest => 'eqsel',
@@ -1180,7 +1180,7 @@
   oprresult => 'bool', oprcom => '<>(bpchar,bpchar)',
   oprnegate => '=(bpchar,bpchar)', oprcode => 'bpcharne', oprrest => 'neqsel',
   oprjoin => 'neqjoinsel' },
-{ oid => '1058', descr => 'less than',
+{ oid => '1058', oid_symbol => 'BpcharLessOperator', descr => 'less than',
   oprname => '<', oprleft => 'bpchar', oprright => 'bpchar',
   oprresult => 'bool', oprcom => '>(bpchar,bpchar)',
   oprnegate => '>=(bpchar,bpchar)', oprcode => 'bpcharlt',
@@ -1195,7 +1195,7 @@
   oprresult => 'bool', oprcom => '<(bpchar,bpchar)',
   oprnegate => '<=(bpchar,bpchar)', oprcode => 'bpchargt',
   oprrest => 'scalargtsel', oprjoin => 'scalargtjoinsel' },
-{ oid => '1061', descr => 'greater than or equal',
+{ oid => '1061', oid_symbol => 'BpcharGreaterEqualOperator', descr => 'greater than or equal',
   oprname => '>=', oprleft => 'bpchar', oprright => 'bpchar',
   oprresult => 'bool', oprcom => '<=(bpchar,bpchar)',
   oprnegate => '<(bpchar,bpchar)', oprcode => 'bpcharge',
@@ -2330,7 +2330,7 @@
   oprresult => 'numeric', oprcode => 'numeric_uplus' },

 # bytea operators
-{ oid => '1955', descr => 'equal',
+{ oid => '1955', oid_symbol => 'ByteaEqualOperator', descr => 'equal',
   oprname => '=', oprcanmerge => 't', oprcanhash => 't', oprleft => 'bytea',
   oprright => 'bytea', oprresult => 'bool', oprcom => '=(bytea,bytea)',
   oprnegate => '<>(bytea,bytea)', oprcode => 'byteaeq', oprrest => 'eqsel',
@@ -2339,7 +2339,7 @@
   oprname => '<>', oprleft => 'bytea', oprright => 'bytea', oprresult => 'bool',
   oprcom => '<>(bytea,bytea)', oprnegate => '=(bytea,bytea)',
   oprcode => 'byteane', oprrest => 'neqsel', oprjoin => 'neqjoinsel' },
-{ oid => '1957', descr => 'less than',
+{ oid => '1957', oid_symbol => 'ByteaLessOperator', descr => 'less than',
   oprname => '<', oprleft => 'bytea', oprright => 'bytea', oprresult => 'bool',
   oprcom => '>(bytea,bytea)', oprnegate => '>=(bytea,bytea)',
   oprcode => 'bytealt', oprrest => 'scalarltsel',
@@ -2354,7 +2354,7 @@
   oprcom => '<(bytea,bytea)', oprnegate => '<=(bytea,bytea)',
   oprcode => 'byteagt', oprrest => 'scalargtsel',
   oprjoin => 'scalargtjoinsel' },
-{ oid => '1960', descr => 'greater than or equal',
+{ oid => '1960', oid_symbol => 'ByteaGreaterEqualOperator', descr => 'greater than or equal',
   oprname => '>=', oprleft => 'bytea', oprright => 'bytea', oprresult => 'bool',
   oprcom => '<=(bytea,bytea)', oprnegate => '<(bytea,bytea)',
   oprcode => 'byteage', oprrest => 'scalargesel',
@@ -2416,7 +2416,7 @@
   oprresult => 'timestamp', oprcode => 'timestamp_mi_interval' },

 # character-by-character (not collation order) comparison operators for character types
-{ oid => '2314', descr => 'less than',
+{ oid => '2314', oid_symbol => 'TextPatternLessOperator', descr => 'less than',
   oprname => '~<~', oprleft => 'text', oprright => 'text', oprresult => 'bool',
   oprcom => '~>~(text,text)', oprnegate => '~>=~(text,text)',
   oprcode => 'text_pattern_lt', oprrest => 'scalarltsel',
@@ -2426,7 +2426,7 @@
   oprcom => '~>=~(text,text)', oprnegate => '~>~(text,text)',
   oprcode => 'text_pattern_le', oprrest => 'scalarlesel',
   oprjoin => 'scalarlejoinsel' },
-{ oid => '2317', descr => 'greater than or equal',
+{ oid => '2317', oid_symbol => 'TextPatternGreaterEqualOperator', descr => 'greater than or equal',
   oprname => '~>=~', oprleft => 'text', oprright => 'text', oprresult => 'bool',
   oprcom => '~<=~(text,text)', oprnegate => '~<~(text,text)',
   oprcode => 'text_pattern_ge', oprrest => 'scalargesel',
@@ -2437,7 +2437,7 @@
   oprcode => 'text_pattern_gt', oprrest => 'scalargtsel',
   oprjoin => 'scalargtjoinsel' },

-{ oid => '2326', descr => 'less than',
+{ oid => '2326', oid_symbol => 'BpcharPatternLessOperator', descr => 'less than',
   oprname => '~<~', oprleft => 'bpchar', oprright => 'bpchar',
   oprresult => 'bool', oprcom => '~>~(bpchar,bpchar)',
   oprnegate => '~>=~(bpchar,bpchar)', oprcode => 'bpchar_pattern_lt',
@@ -2447,7 +2447,7 @@
   oprresult => 'bool', oprcom => '~>=~(bpchar,bpchar)',
   oprnegate => '~>~(bpchar,bpchar)', oprcode => 'bpchar_pattern_le',
   oprrest => 'scalarlesel', oprjoin => 'scalarlejoinsel' },
-{ oid => '2329', descr => 'greater than or equal',
+{ oid => '2329', oid_symbol => 'BpcharPatternGreaterEqualOperator', descr => 'greater than or equal',
   oprname => '~>=~', oprleft => 'bpchar', oprright => 'bpchar',
   oprresult => 'bool', oprcom => '~<=~(bpchar,bpchar)',
   oprnegate => '~<~(bpchar,bpchar)', oprcode => 'bpchar_pattern_ge',