MIME-Version: 1.0
References: <a5313e34-3fa2-4d66-ba88-e4df9ea3b4af@vondra.me>
 <202507071221.am6rbd6n4dct@alvherre.pgsql>
 <CAE7r3MJeqDSfic9pQedsRg3xg49q7=iq5PTuZNNNi7yqAw8dTw@mail.gmail.com>
 <67115399-4799-4c3b-bd17-d43c98099919@vondra.me>
In-Reply-To: <67115399-4799-4c3b-bd17-d43c98099919@vondra.me>
From: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>
Date: Wed, 9 Jul 2025 09:58:57 +0300
Message-ID: 
 <CAE7r3MLDcDjh5ieGS=UNtKVm6wG0mRRqdYGUEoKvsDoFp9PgRQ@mail.gmail.com>
Subject: Re: amcheck support for BRIN indexes
To: Tomas Vondra <tomas@vondra.me>
Cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAE7r3MLDcDjh5ieGS%3DUNtKVm6wG0mRRqdYGUEoKvsDoFp9PgRQ%40mail.gmail.com>
Precedence: bulk

On Tue, Jul 8, 2025 at 4:21=E2=80=AFPM Tomas Vondra <tomas@vondra.me> wrote=
:
>
>
>
> On 7/8/25 14:40, Arseniy Mukhin wrote:
> > On Mon, Jul 7, 2025 at 3:21=E2=80=AFPM =C3=81lvaro Herrera <alvherre@ku=
rilemu.de> wrote:
> >>
> >> On 2025-Jul-07, Tomas Vondra wrote:
> >>
> >>> Alvaro, what's your opinion on the introduction of the new WITHIN_RAN=
GE?
> >>> 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
> > (=D1=81), it seems most of the time to be really trivial to build such =
a
> > ScanKey, but not every opclass supports '=3D' 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
> > '=3D' 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 the=
n?
> >
>
> 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 ...

This doesn't change much, but it seems we need an array of operators
in the case of a multi-column index. And one more thing, it's
theoretically possible that opclass doesn't have the operator we need
at all. I'm not aware of such cases, but perhaps in the future
something like GIN tsvector_ops could be created for BRIN.
tsvector_ops doesn't have an operator that could be used here.


Best regards,
Arseniy Mukhin