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
>From 4ee5e2bbd0c44c82d1604836db5352f3e0e0fbf2 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Mon, 22 Mar 2021 20:36:59 +0100
Subject: [PATCH 1/5] fixup opclass storage type
---
src/backend/commands/opclasscmds.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index fad39e2b75..78b2d69782 100644
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -576,10 +576,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
*/
if (OidIsValid(storageoid))
{
- /* Just drop the spec if same as column datatype */
- if (storageoid == typeoid)
- storageoid = InvalidOid;
- else if (!amstorage)
+ if ((storageoid != typeoid) && (!amstorage))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("storage type cannot be different from data type for access method \"%s\"",
--
2.30.2
