Handling of opckeytype / CREATE OPERATOR CLASS (bug?)

Hi,

while working on the new BRIN opclasses in [1], I stumbled on something
I think is actually a bug in how CREATE OPERATOR CLASS handles the
storage type. If you look at built-in opclasses, there's a bunch of
cases where (opcintype == opckeytype), for example the BRIN opclasses
for inet data type:

test=# select oid, opcname, opcfamily, opcintype, opckeytype from
pg_opclass where opcintype = 869 order by oid;

  oid  |        opcname        | opcfamily | opcintype | opckeytype
-------+-----------------------+-----------+-----------+------------
 10105 | inet_minmax_ops       |      4075 |       869 |        869
 10106 | inet_inclusion_ops    |      4102 |       869 |        869

The fact that opckeytype is set is important, because this allows the
opclass to handle data with cidr data type - there's no opclass for
cidr, but we can do this:

create table t (a cidr);
insert into t values ('127.0.0.1');
insert into t values ('127.0.0.2');
insert into t values ('127.0.0.3');
create index on t using brin (a inet_minmax_ops);

This seems to work fine. Now, if you manually update the opckeytype to
0, you'll get this:

test=# update pg_opclass set opckeytype = 0 where oid = 10105;
UPDATE 1
test=# create index on t using brin (a inet_minmax_ops);
ERROR:  missing operator 1(650,650) in opfamily 4075


Unfortunately, it turns out it's impossible to re-create this opclass
using CREATE OPERATOR CLASS, because the opclasscmds.c does this:

    /* Just drop the spec if same as column datatype */
    if (storageoid == typeoid && false)
        storageoid = InvalidOid;

So the storageoid is reset to 0. This only works for the built-in
opclasses because those are injected directly into the catalogs.

Either the CREATE OPERATOR CLASS is not considering something before
resetting the storageoid, or maybe it should be reset for all opclasses
(including the built-in ones) and the code that's using it should
restore it in those cases (AFAICS opckeytype=0 means it's the same as
opcintkey).

Attached is a PoC patch doing the first thing - this does the trick for
me, but I'm not 100% sure it's the right fix.


[1]
https://www.postgresql.org/message-id/54b47e66-bd8a-d44a-2090-fd4b2f49abe6%40enterprisedb.com

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company