Re: [PATCH] Add sortsupport for range types and btree_gist

Am Montag, dem 12.02.2024 um 21:00 +0800 schrieb jian he:
> + <para>
> +  Per default <filename>btree_gist</filename> builts
> <acronym>GiST</acronym> indexe with
> +  <function>sortsupport</function> in <firstterm>sorted</firstterm>
> mode. This usually results in a
> +  much better index quality and smaller index sizes by much faster
> index built speed. It is still
> +  possible to revert to buffered built strategy by using the
> <literal>buffering</literal> parameter
> +  when creating the index.
> + </para>
> +
> I believe `built` |`builts` should be `build`.

Right, Fixed.

> Also
> maybe we can simply copy some texts from
> https://www.postgresql.org/docs/current/gist-implementation.html.
> how about the following:
>   <para>
>    The sorted method is only available if each of the opclasses used
> by the
>    index provides a <function>sortsupport</function> function, as
> described
>    in <xref linkend="gist-extensibility"/>.  If they do, this method
> is
>    usually the best, so it is used by default.
>   It is still possible to change to a buffered build strategy by
> using
> the <literal>buffering</literal> parameter
>   to the CREATE INDEX command.
>   </para>

Hmm not sure what you are trying to achieve with this? The opclasses in
btree_gist provides sortsupport, but by reading the above i would get
the impression they're still optional.

>
> you've changed contrib/btree_gist/meson.build, seems we also need to
> change contrib/btree_gist/Makefile
>

Oh, good catch. I'm so focused on meson already that i totally forgot
the good old Makefile. Fixed.

> gist_point_sortsupport have `if (ssup->abbreviate)`,  does
> range_gist_sortsupport also this part?
> I think the `if(ssup->abbreviate)` part is optional?
> Can we add some comments on it?

I've thought about abbreviated keys support but put that aside for
later. I wanted to focus on general sortsupport first before getting my
hands on it and so postponed it for another round.

If we agree that this patch needs support for abbreviated keys now, i
certainly can work on it.

Thanks for your review,

Bernd