Subject: Re: WIP: BRIN multi-range indexes
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: John Naylor <john.naylor@enterprisedb.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <20210303001725.GA1766@alvherre.pgsql>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
Message-ID: <23a0a854-8f1a-e965-7121-f76b1b2fc271@enterprisedb.com>
Date: Wed, 3 Mar 2021 02:17:20 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <20210303001725.GA1766@alvherre.pgsql>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/3/21 1:17 AM, Alvaro Herrera wrote:
> On 2021-Mar-03, Tomas Vondra wrote:
> 
>> That's kinda my point - I agree the size of the patch is not the
>> primary concern, but it makes the minmax/inclusion code a bit more
>> complicated (because they now have to loop over the keys), with
>> very little benefit (there might be some speedup, but IMO it's
>> rather negligible).
> 
> Yeah, OK.
> 
>> Alternatively we could simply remove the code supporting the old
>> API with "consistent" functions without the additional parameter.
>> But the idea was to seamlessly support existing opclasses / not
>> breaking them unnecessarily (I know we don't guarantee that in
>> major upgrades, but as they may not benefit from this, why break
>> them?). It'd simplify the code in brin.c a little bit, but the
>> opclasses a bit more complex.
> 
> Well, I doubt any opclass-support functions exist outside of core. Or
> am I just outdated and we do know of some?
> 

I'm not aware of any. This was proposed by Nikita Glukhov:

https://www.postgresql.org/message-id/49cb668f-d6f9-3493-681d-7d40b715ef64%40postgrespro.ru

Alexander Korotkov seemed to agree with Nikita, but I don't recall any
references to actual existing opclasses.

Then again - the amount of code to support two signatures is fairly
small. If we decide to get rid of it, then fine - but the new complexity
in minmax/inclusion likely exceeds that.

Which is why I was thinking about still supporting both signatures, but
reverting the changes in brin_minmax and brin_inclusion.


regards

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