Subject: Re: WIP: BRIN multi-range indexes
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: John Naylor <john.naylor@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <20210303001725.GA1766@alvherre.pgsql>
 <23a0a854-8f1a-e965-7121-f76b1b2fc271@enterprisedb.com>
 <cf8fca26-0fdd-ac74-5dac-66a2ccb59f56@enterprisedb.com>
 <8e319879-32ff-1eb8-d7d3-6a3ac8a51728@enterprisedb.com>
 <CAFBsxsHrqP+OMp5cV6yiEzkwobtAp4rnxOGsfm_s_vX8qSNQeQ@mail.gmail.com>
 <35e866e9-d820-07de-b2be-69d5ac479179@enterprisedb.com>
 <71dfd257-c5ce-530d-90d3-7c744a28e97b@enterprisedb.com>
 <CAFBsxsHswPBXQvvW87uSR59MkPNhEB5Txe4sFjVBfE9TR4jrGw@mail.gmail.com>
 <ea369ea5-0d61-bbee-c1c7-bfe17728bab1@enterprisedb.com>
Message-ID: <54b47e66-bd8a-d44a-2090-fd4b2f49abe6@enterprisedb.com>
Date: Mon, 22 Mar 2021 10:10:11 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.0
MIME-Version: 1.0
In-Reply-To: <ea369ea5-0d61-bbee-c1c7-bfe17728bab1@enterprisedb.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/54b47e66-bd8a-d44a-2090-fd4b2f49abe6%40enterprisedb.com>
Precedence: bulk

On 3/22/21 6:27 AM, Tomas Vondra wrote:
> 
> ...
>
> All the regression tests work fine, with the exception of minmax-multi
> on a CIDR column. I don't know why, but the CREATE INDEX then fails like
> this:
> 
>   ERROR:  missing operator 1(650,650) in opfamily 16463
> 
> 650 is cidr, so this essentially says there's no (cidr<cidr) operator.
> With the opclasses defined in .dat files this however worked, so I
> suppose it's related to the missing operator families.
> 

Turns out this is likely a bug elsewhere. After a couple fixes to the
extension SQL script, the only real difference in catalog contents
(compared to e.g. the built-in BRIN minmax inet opclass) is that the
built-in one has opckeytype set to 869 (i.e. inet), while the one
created from extension has it set to 0.

The opclasses for inet (OID 869) look like this:

test=# select oid, opcname, opcfamily, opcintype, opckeytype from
pg_opclass where opcintype = 869 order by oid;
  oid  |        opcname        | opcfamily | opcintype | opckeytype
-------+-----------------------+-----------+-----------+------------
 10009 | cidr_ops              |      1974 |       869 |          0
 10010 | cidr_ops              |      1975 |       869 |          0
 10015 | inet_ops              |      1974 |       869 |          0
 10016 | inet_ops              |      1975 |       869 |          0
 10017 | inet_ops              |      3550 |       869 |          0
 10018 | inet_ops              |      3794 |       869 |          0
 10105 | inet_minmax_ops       |      4075 |       869 |        869
 10106 | inet_inclusion_ops    |      4102 |       869 |        869
 16451 | inet_bloom_ops        |     16450 |       869 |          0
 17398 | inet_minmax_multi_ops |     17397 |       869 |          0
(10 rows)

The only difference between the two minmax variants is the opckeytype,
and if I update it to 869 for inet_minmax_multi_ops, it starts working.
Likewise, if I set it to 0 for inet_minmax_ops, it breaks the same way.

Turns out, this is impossible to set from CREATE OPERATOR CLASS, because
opclasscmds.c does this:

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

but index.c looks only at opclassTup->opckeytype. The built-in opclasses
don't have this issue, because they put the data into the catalog
directly, without going through this code.

I don't know what's the right fix, but it seems like this patch has
nothing to do with it. If we want to move the opclasses into an
extension, we can comment out that one (cidr/inet) case for now.


regards

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