Re: Yet another fast GiST build

On 21/10/2020 20:13, Andrey Borodin wrote:
>> 7 окт. 2020 г., в 17:38, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> написал(а):
>>
>> On 07/10/2020 15:27, Andrey Borodin wrote:
>>> Here's draft patch with implementation of sortsupport for ints and floats.
>>
>>> +static int
>>> +gbt_int4_cmp(Datum a, Datum b, SortSupport ssup)
>>> +{
>>> + int32KEY *ia = (int32KEY *) DatumGetPointer(a);
>>> + int32KEY *ib = (int32KEY *) DatumGetPointer(b);
>>> +
>>> + if (ia->lower == ib->lower)
>>> + {
>>> + if (ia->upper == ib->upper)
>>> + return 0;
>>> +
>>> + return (ia->upper > ib->upper) ? 1 : -1;
>>> + }
>>> +
>>> + return (ia->lower > ib->lower) ? 1 : -1;
>>> +}
>>
>> We're only dealing with leaf items during index build, so the 'upper' and 'lower' should always be equal here, right? Maybe add a comment and an assertion on that.
>>
>> (It's pretty sad that the on-disk representation is identical for leaf and internal items, because that wastes a lot of disk space, but that's way out of scope for this patch.)
>
> Thanks, I've added assert() where is was easy to test equalty.
>
> PFA patch with all types.

gbt_ts_cmp(), gbt_time_cmp_sort() and gbt_date_cmp_sort() still have the
above issue, they still compare "upper" for no good reason.

> +static Datum
> +gbt_bit_abbrev_convert(Datum original, SortSupport ssup)
> +{
> + return (Datum) 0;
> +}
> +
> +static int
> +gbt_bit_cmp_abbrev(Datum z1, Datum z2, SortSupport ssup)
> +{
> + return 0;
> +}

If an abbreviated key is not useful, just don't define abbrev functions
and don't set SortSupport->abbrev_converter in the first place.

> 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?

> I had a plan to implement and test one type each day. I did not quite understood how rich our type system is.

:-)

- Heikki