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',
