Re: [PATCH] Add sortsupport for range types and btree_gist - Mailing list pgsql-hackers

From Bernd Helmle
Subject Re: [PATCH] Add sortsupport for range types and btree_gist
Date
Msg-id b764bf2ababe2066128b5e8977e6db0990fa1fe9.camel@oopsware.de
Whole thread Raw
In response to Re: [PATCH] Add sortsupport for range types and btree_gist  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: [PATCH] Add sortsupport for range types and btree_gist
List pgsql-hackers
Am Mittwoch, dem 02.04.2025 um 22:41 +0300 schrieb Heikki Linnakangas:
> * The stuff to save the FmgrInfo for gbt_enum_sortsupport() was
> broken.
> It saved the address of FmgrInfo struct that was allocated in a local
> variable, on the stack, in SortSupport->ssup-extra, and expected it
> to
> be valid on subsequent call to gbt_enum_ssup_cmp(). It went unnoticed
> because enum_cmp() only accesses the FmgrInfo struct if it encounters
> odd enum values, i.e. enum values that have been added with ALTER
> TYPE
> ADD VALUE. I fixed that, and modified the existing enum test to cover
> that case.
>

Ugh, i obviously didn't pay enough attention here.

> * The gist_bpchar_ops opfamily was using the built-in
> bpchar_sortsupport() function directly. That's not right, you need to
> have a shim that extracts and comparse just the 'lower' value, just
> like
> for all other datatypes. I think that was just a simple oversight,
> but
> it happened to pass the tests because bpchar_sortsupport() would not
> outright crash, even though we were passing it garbage. It's
> interesting
> that because the gist sortsupport function is used just to order the
> input during build, everything still works if it sorts the input to a
> completely bogus ordering, it just gets slower. At one point while
> fixing that, I also accidentally used "btcharcmp" instead of
> "bpcharcmp", and all the tests passed with that too.
>

Yes, i missed that. Originally the code did this all over the place
with many other types, too. I seem to have overseen this somehow.

> Those are now fixed. I also harmonized the comments to use the same
> phrasing for all the datatypes, marked all the sortsupport functions
> as
> PARALLEL SAFE, and reformatted slightly.

Note that the original coding was by Christoph Heiss and i picked up
his work and rewrote/fixed especially the part with the direct use of
sortsupport functions and many other issues spotted by testing and by
Andrey.

Many thanks for picking this up. There are users out there which are
going to be very happy with this if no further issues arise. :)





pgsql-hackers by date:

Previous
From: "Euler Taveira"
Date:
Subject: Re: why there is not VACUUM FULL CONCURRENTLY?
Next
From: Peter Eisentraut
Date:
Subject: Re: Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints