Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> Here is a draft patch.
> It will require a catversion change because those operator classes don't
> have assigned OIDs so far.
That's slightly annoying given where we are with v12. We could
avoid it by looking up the opclass's opfamily and seeing if it's
TEXT_BTREE_FAM_OID etc, which do already have hand-assigned OIDs.
But maybe avoiding a catversion bump now is not worth the cost of
an extra syscache lookup. (It'd give me an excuse to shove the
leakproofness-marking changes from the other thread into v12, so
there's that.)
Speaking of extra syscache lookups, I don't like that you rearranged
the if-test to check nondeterminism before the opclass identity checks.
That's usually going to be a wasted lookup.
> The comment block I just moved over for the time being. It should
> probably be rephrased a bit.
Indeed. Maybe like
* text_pattern_ops uses text_eq as the equality operator, which is
* fine as long as the collation is deterministic; text_eq then
* reduces to bitwise equality and so it is semantically compatible
* with the other operators and functions in the opclass. But with a
* nondeterministic collation, text_eq could yield results that are
* incompatible with the actual behavior of the index (which is
* determined by the opclass's comparison function). We prevent
* such problems by refusing creation of an index with this opclass
* and a nondeterministic collation.
*
* The same applies to varchar_pattern_ops and bpchar_pattern_ops.
* If we find more cases, we might decide to create a real mechanism
* for marking opclasses as incompatible with nondeterminism; but
* for now, this small hack suffices.
*
* Another solution is to use a special operator, not text_eq, as the
* equality opclass member, but that is undesirable because it would
* prevent index usage in many queries that work fine today.
regards, tom lane