Message-ID: <67115399-4799-4c3b-bd17-d43c98099919@vondra.me>
Date: Tue, 8 Jul 2025 15:21:04 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: amcheck support for BRIN indexes
To: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>,
 =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <a5313e34-3fa2-4d66-ba88-e4df9ea3b4af@vondra.me>
 <202507071221.am6rbd6n4dct@alvherre.pgsql>
 <CAE7r3MJeqDSfic9pQedsRg3xg49q7=iq5PTuZNNNi7yqAw8dTw@mail.gmail.com>
Content-Language: en-US
From: Tomas Vondra <tomas@vondra.me>
In-Reply-To: 
 <CAE7r3MJeqDSfic9pQedsRg3xg49q7=iq5PTuZNNNi7yqAw8dTw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/67115399-4799-4c3b-bd17-d43c98099919%40vondra.me>
Precedence: bulk


On 7/8/25 14:40, Arseniy Mukhin wrote:
> On Mon, Jul 7, 2025 at 3:21 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>
>> On 2025-Jul-07, Tomas Vondra wrote:
>>
>>> Alvaro, what's your opinion on the introduction of the new WITHIN_RANGE?
>>> I'd probably try to do this using the regular consistent function:
>>>
>>> (a) we don't need to add stuff to all BRIN opclasses to support this
>>>
>>> (b) it gives us additional testing of the consistent function
>>>
>>> (c) building a scan key for equality seems pretty trivial
>>>
>>> What do you think?
>>
>> Oh yeah, if we can build this on top of the existing primitives, then
>> I'm all for that.
> 
> Thank you for the feedback! I agree with the benefits. Speaking of
> (с), it seems most of the time to be really trivial to build such a
> ScanKey, but not every opclass supports '=' operator. amcheck should
> handle these cases somehow then. I see two options here. The first is
> to not provide  'heap all indexed' check for such opclasses, which is
> sad because even one core opclass (box_inclusion_ops) doesn't support
> '=' operator, postgis brin opclasses don't support it too AFAICS. The
> second option is to let the user define which operator to use during
> the check, which, I think, makes user experience much worse in this
> case. So both options look not good from the user POV as for me, so I
> don't know. What do you think about it?
> 
> And should I revert the patchset to the consistent function version then?
> 

Yeah, that's a good point. The various opclasses may support different
operators, and we don't know which "strategy" to fill into the scan key.
Minmax needs BTEqualStrategyNumber, inclusion RTContainsStrategyNumber,
and so on.

I wonder if there's a way to figure this out from the catalogs, but I
can't think of anything. Maybe requiring the user to supply the operator
(and not checking heap if it's not provided)

  SELECT brin_index_check(..., '@>');

would not be too bad ...


Alvaro, any ideas?


-- 
Tomas Vondra