On 30/10/2020 20:20, Andrey Borodin wrote:
27 окт. 2020 г., в 16:43, Heikki Linnakangas <hlinn...@iki.fi> написал(а):
static bool
gbt_inet_abbrev_abort(int memtupcount, SortSupport ssup)
{
#if SIZEOF_DATUM == 8
        return false;
#else
        return true;
#endif
}

Better to not set the 'abbrev_converter' function in the first place. Or would 
it be better to cast the float8 to float4 if SIZEOF_DATUM == 4?
Ok, now for 4 bytes Datum we do return (Datum) Float4GetDatum((float) z);

A few quick comments:

* Currently with float8, you immediately abort abbreviation if SIZEOF_DATUM == 4. Like in the 'inet' above, you could convert the float8 to float4 instead.

* Some of the ALTER OPERATOR FAMILY commands in btree_gist--1.6--1.7.sql are copy-pasted wrong. Here's one example:

ALTER OPERATOR FAMILY gist_timestamptz_ops USING gist ADD
        FUNCTION        11      (text, text) gbt_text_sortsupport;

* It's easy to confuse the new comparison functions with the existing comparisons used by the picksplit functions. Some comments and/or naming changes would be good to clarify how they're different.

* It would be straightforward to have abbreviated functions for macaddr and macaddr8 too.

How do you think, should this patch and patch with pageinspect GiST functions 
be registered on commitfest?

Yeah, that'd be good.

- Heikki


Reply via email to