Re: Handling of opckeytype / CREATE OPERATOR CLASS (bug?)

Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> writes:
> On 3/23/21 6:15 AM, Tom Lane wrote:
>> Digging in our git history, the rule about zero opckeytype dates to
>> 2001 (f933766ba), which precedes our invention of polymorphic types
>> in 2003 (somewhere around 730840c9b). So I'm pretty sure that that
>> was a poor man's substitute for polymorphic opclasses, which we
>> failed to clean up entirely after we got real polymorphic opclasses.

> That's possible. I'm not familiar with how we deal with polymorphic
> opclasses etc. but I tried to look for places dealing with opckeytype,
> so that I can compare BRIN vs. the other AMs, but the only references
> seem to be in amvalidate() functions.
> So either the difference is not very obvious, or maybe the other AMs
> don't trigger this for some reason. For example btree has a separate
> opclass for cidr, so it does not have to use "inet_ops" as polymorphic.

I think the difference is that brin is trying to look up opclass members
based on the recorded type of the index's column (not the underlying
table column). I don't recall that anyplace else does that. btree
for instance does some lookups based on opcintype, but I don't think
it looks at the index column type anywhere.

After poking at it a bit more, the convention for zero does allow us
to do some things that regular polymorphism won't. As an example:

test=# create table vc (id varchar(9) primary key);
CREATE TABLE
test=# \d+ vc_pkey
Index "public.vc_pkey"
Column | Type | Key? | Definition | Storage | Stats target
--------+----------------------+------+------------+----------+--------------
id | character varying(9) | yes | id | extended |
primary key, btree, for table "public.vc"

If btree text_ops had opckeytype = 'text' then this index column
would show as just "text", which while not fatal seems like a loss
of information.

So I'm coming around to the idea that opckeytype = opcintype and
opckeytype = 0 are valid but distinct situations, and CREATE OPCLASS
indeed ought not smash one to the other. But we'd better poke around
at the documentation, pg_dump, etc and make sure everything plays
nice with that.

regards, tom lane