Re: [PATCH] ltree hash functions

On Thu, Feb 1, 2024 at 11:11 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, 6 Dec 2023 at 04:08, Tommy Pavlicek <tommypav122(at)gmail(dot)com> wrote:
> >
> > Thanks.
> >
> > I've attached the latest version that updates the naming in line with
> > the convention.
> >
> > On Mon, Dec 4, 2023 at 12:46 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Dec 1, 2023 at 8:44 AM Tommy Pavlicek <tommypav122(at)gmail(dot)com> wrote:
> > > >
> > > >
> > > > Patch updated for those comments (and a touch of cleanup in the tests) attached.
> > >
> > > it would be a better name as hash_ltree than ltree_hash, similar logic
> > > applies to ltree_hash_extended.
> > > that would be the convention. see: https://stackoverflow.com/a/69650940/15603477
> > >
> > >
> > > Other than that, it looks good.
>
> CFBot shows one of the test is failing as in [1]:
> diff -U3 /tmp/cirrus-ci-build/contrib/ltree/expected/ltree.out
> /tmp/cirrus-ci-build/build-32/testrun/ltree/regress/results/ltree.out
> --- /tmp/cirrus-ci-build/contrib/ltree/expected/ltree.out 2024-01-31
> 15:18:42.893039599 +0000
> +++ /tmp/cirrus-ci-build/build-32/testrun/ltree/regress/results/ltree.out
> 2024-01-31 15:23:25.309028749 +0000
> @@ -1442,9 +1442,14 @@
> ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v)
> WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32)
> OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32);
> - value | standard | extended0 | extended1
> --------+----------+-----------+-----------
> -(0 rows)
> + value | standard |
> extended0 | extended1
> +-------------+----------------------------------+----------------------------------+----------------------------------
> + 0 | 10001010100010010000000000001011 |
> 01011001111001000100011001011011 | 01011001111001000100011010011111
> + 0.1 | 10100000111110001010110001001110 |
> 00111100100010001100110111010101 | 00111100100010001101100011010101
> + 0.1.2 | 01111000011100000101111101110100 |
> 10101110011101011000000011010111 | 10101110011101110010001111000011
> + 0 | 10001010100010010000000000001011 |
> 01011001111001000100011001011011 | 01011001111001000100011010011111
> + 0_asd.1_ASD | 01000010001010000000101001001101 |
> 00111100100010001100110111010101 | 00111100100010001101100011010101
> +(5 rows)
>
> Please post an updated version for the same.
>
> [1] - https://api.cirrus-ci.com/v1/artifact/task/5572544858685440/testrun/build-32/testrun/ltree/regress/regression.diffs
>

It only fails on Linux - Debian Bullseye - Meson.
I fixed the white space, named it v5.
I also made the following changes:
from

uint64 levelHash = hash_any_extended((unsigned char *) al->name, al->len, seed);
uint32 levelHash = hash_any((unsigned char *) al->name, al->len);

to
uint64 levelHash = DatumGetUInt64(hash_any_extended((unsigned char *)
al->name, al->len, seed));
uint32 levelHash = DatumGetUInt32(hash_any((unsigned char *) al->name,
al->len));

(these two line live in different functions)

I have some problems testing it locally, so I post the patch.