Handling of opckeytype / CREATE OPERATOR CLASS (bug?)

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Handling of opckeytype / CREATE OPERATOR CLASS (bug?)
Date: 2021-03-23 01:34:57
Message-ID: 2496d45e-b412-2a55-a0d6-80d160492579@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-fixup-opclass-storage-type-20210323.patch text/x-patch 967 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2021-03-23 01:40:33 Re: New IndexAM API controlling index vacuum strategies
Previous Message Fujii Masao 2021-03-23 01:16:05 Re: Type of wait events WalReceiverWaitStart and WalSenderWaitForWAL