MIME-Version: 1.0
References: <CAJ7c6TMPhDRQMmkUHPv8oOK97B1mR8NRS61DgjpdaZUPAwaeZQ@mail.gmail.com>
 <CANWCAZYHK4F1MPBytEKSS8qhi9kiUXhJTZq-rWcyzk6BCOyfYg@mail.gmail.com> <CAJ7c6TPZVKk1Dn7LWRatBi=UFYfEPmfGGFYT8wQzDj7y8oJ_CQ@mail.gmail.com>
In-Reply-To: <CAJ7c6TPZVKk1Dn7LWRatBi=UFYfEPmfGGFYT8wQzDj7y8oJ_CQ@mail.gmail.com>
From: John Naylor <johncnaylorls@gmail.com>
Date: Mon, 23 Feb 2026 16:40:28 +0700
Message-ID: <CANWCAZYWdOEnoL_88VpMge1RtRpBz-VRCjdcu-eA4q3U6LvpDw@mail.gmail.com>
Subject: Re: [PATCH] Refactor *_abbrev_convert() functions
To: Aleksander Alekseev <aleksander@tigerdata.com>
Cc: PostgreSQL Development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CANWCAZYWdOEnoL_88VpMge1RtRpBz-VRCjdcu-eA4q3U6LvpDw%40mail.gmail.com>
Precedence: bulk

On Tue, Feb 3, 2026 at 9:56=E2=80=AFPM Aleksander Alekseev
<aleksander@tigerdata.com> wrote:
> > There's more we can do here. Above the stanzas changed in the patch
> > there is this, at least for varlena/bytea:
> >
> > hash =3D DatumGetUInt32(hash_any((unsigned char *) authoritative_data,
> >                       Min(len, PG_CACHE_LINE_SIZE)));
> >
> > This makes no sense to me: hash_any() calls hash_bytes() and turns the
> > result into a Datum, and then we just get it right back out of the
> > Datum again.
>
> I see similar patterns in files other than bytea.c and varlena.c.
> Implemented as a separate patch.

I think it makes sense to squash 0001 and 0003 together, then 0002 and
0004 together.

For the first, we should probably combine in the upper half when using
a 64-bit hash, like this:

     /* Hash abbreviated key */
     {
-        uint32          tmp;
+        uint64          tmp;

-        tmp =3D DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32)=
;
-        hash =3D DatumGetUInt32(hash_uint32(tmp));
+        tmp =3D murmurhash64(DatumGetUInt64(res));
+        hash =3D (uint32) tmp ^ tmp >> 32;
     }

> Using hash_bytes_uint32() / hash_bytes_uint32_extended() directly in
> timetz_hash() / timetz_hash_extended() is safe though. Proposed as a
> separate patch.

0005 doesn't buy us as much in readability since the two lines no longer ma=
tch.

Further cleanup possible now that we have 64-bit datums: MAC addresses
are always 6 bytes, so abbreviation is no longer relevant -- datum1 is
authoritative. That's in scope for the thread subject but also a
bigger patch, but maybe someone would like to pick it up for PG20.

--
John Naylor
Amazon Web Services