Re: 64-bit hash function for hstore and citext data type

Thanks Tom for enhancing & committing these patches.

Regards,
Amul
On Sat, Nov 24, 2018 at 12:15 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> > "Tomas" == Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> > Tomas> The important question is - can there be two different encodings
> > Tomas> for the same hstore value?
>
> > I was going to say "no", but in fact on closer examination there is an
> > edge case caused by the fact that hstoreUpgrade allows an _empty_ hstore
> > from pg_upgraded 8.4 data through without modifying it. (There's also a
> > vanishingly unlikely case involving the pgfoundry release of hstore-new.)
>
> Ugh. Still, that's a pre-existing problem in hstore_hash, and so I don't
> think it's a blocker for this patch.
>
> > I'm inclined to fix this in hstoreUpgrade rather than complicate
> > hstore_hash with historical trivia. Also there have been no field
> > complaints - I guess it's unlikely that there is much pg 8.4 hstore data
> > in the wild that anyone wants to hash.
>
> Changing hstoreUpgrade at this point seems like wasted/misguided effort.
> I don't doubt that there was a lot of 8.4 hstore data out there, but how
> much remains unmigrated? If we're going to take this seriously at all,
> my inclination would be to change hstore_hash[_extended] to test for
> the empty-hstore case and force the same value it gets for such an
> hstore made today.
>
> In the meantime, I went ahead and pushed these patches. The only
> non-cosmetic changes I made were to remove the changes in
> citext--unpackaged--1.0.sql/hstore--unpackaged--1.0.sql; those
> were wrong, because the point of those files is to migrate pre-9.1
> databases into the extension system. Such a database would not
> contain an extended hash function, and so adding an ALTER EXTENSION
> command for that function would cause the script to fail.
>
> regards, tom lane