public inbox for [email protected]help / color / mirror / Atom feed
[PATCH] Refactor *_abbrev_convert() functions 3+ messages / 3 participants [nested] [flat]
* [PATCH] Refactor *_abbrev_convert() functions @ 2026-01-13 12:34 Aleksander Alekseev <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Aleksander Alekseev @ 2026-01-13 12:34 UTC (permalink / raw) To: pgsql-hackers Hi, Now when all Datums are 64-bit values we can simplify the code by using murmurhash64(). This refactoring was previously suggested by John Naylor [1]. [1]: https://postgr.es/m/CANWCAZbMyrijdR0xc-4SqpNJBHMEwRZccBK4fa0aquNpq2Uj7w%40mail.gmail.com -- Best regards, Aleksander Alekseev Attachments: [text/x-patch] v1-0001-Refactor-_abbrev_convert-functions.patch (4.5K, 2-v1-0001-Refactor-_abbrev_convert-functions.patch) download | inline diff: From 69f0407ce161af62166b13eb504477d95ed31176 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <[email protected]> Date: Tue, 13 Jan 2026 14:51:21 +0300 Subject: [PATCH v1] Refactor *_abbrev_convert() functions Now when all Datums are 64-bit values we can simplify the code by using murmurhash64(). Author: Aleksander Alekseev <[email protected]> Suggested-by: John Naylor <[email protected]> Reviewed-by: TODO FIXME Discussion: TODO FIXME --- src/backend/utils/adt/bytea.c | 7 +------ src/backend/utils/adt/mac.c | 11 +++-------- src/backend/utils/adt/network.c | 6 +----- src/backend/utils/adt/numeric.c | 5 +---- src/backend/utils/adt/uuid.c | 6 +----- src/backend/utils/adt/varlena.c | 7 +------ 6 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/backend/utils/adt/bytea.c b/src/backend/utils/adt/bytea.c index fd7662d41ee..d880b62688c 100644 --- a/src/backend/utils/adt/bytea.c +++ b/src/backend/utils/adt/bytea.c @@ -1115,12 +1115,7 @@ bytea_abbrev_convert(Datum original, SortSupport ssup) addHyperLogLog(&bss->full_card, hash); /* Hash abbreviated key */ - { - uint32 tmp; - - tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32); - hash = DatumGetUInt32(hash_uint32(tmp)); - } + hash = (uint32) murmurhash64(DatumGetUInt64(res)); addHyperLogLog(&bss->abbr_card, hash); diff --git a/src/backend/utils/adt/mac.c b/src/backend/utils/adt/mac.c index f14675dea40..0658846f274 100644 --- a/src/backend/utils/adt/mac.c +++ b/src/backend/utils/adt/mac.c @@ -492,17 +492,12 @@ macaddr_abbrev_convert(Datum original, SortSupport ssup) uss->input_count += 1; /* - * Cardinality estimation. The estimate uses uint32, so XOR the two 32-bit - * halves together to produce slightly more entropy. The two zeroed bytes - * won't have any practical impact on this operation. + * Cardinality estimation. The estimate uses uint32, so we hash the full + * 64-bit value and take the lower 32 bits of the result. */ if (uss->estimating) { - uint32 tmp; - - tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32); - - addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp))); + addHyperLogLog(&uss->abbr_card, (uint32) murmurhash64(DatumGetUInt64(res))); } /* diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c index 3a2002097dd..c226af5ca80 100644 --- a/src/backend/utils/adt/network.c +++ b/src/backend/utils/adt/network.c @@ -739,11 +739,7 @@ network_abbrev_convert(Datum original, SortSupport ssup) /* Hash abbreviated key */ if (uss->estimating) { - uint32 tmp; - - tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32); - - addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp))); + addHyperLogLog(&uss->abbr_card, (uint32) murmurhash64(DatumGetUInt64(res))); } return res; diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 891ae6ba7fe..6d5e2a3644f 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -2397,10 +2397,7 @@ numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss) if (nss->estimating) { - uint32 tmp = ((uint32) result - ^ (uint32) ((uint64) result >> 32)); - - addHyperLogLog(&nss->abbr_card, DatumGetUInt32(hash_uint32(tmp))); + addHyperLogLog(&nss->abbr_card, (uint32) murmurhash64(result)); } return NumericAbbrevGetDatum(result); diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c index 6ee3752ac78..888802c3012 100644 --- a/src/backend/utils/adt/uuid.c +++ b/src/backend/utils/adt/uuid.c @@ -396,11 +396,7 @@ uuid_abbrev_convert(Datum original, SortSupport ssup) if (uss->estimating) { - uint32 tmp; - - tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32); - - addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp))); + addHyperLogLog(&uss->abbr_card, (uint32) murmurhash64(DatumGetUInt64(res))); } /* diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index c80191f0a22..280f2e65898 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -2115,12 +2115,7 @@ varstr_abbrev_convert(Datum original, SortSupport ssup) addHyperLogLog(&sss->full_card, hash); /* Hash abbreviated key */ - { - uint32 tmp; - - tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32); - hash = DatumGetUInt32(hash_uint32(tmp)); - } + hash = (uint32) murmurhash64(DatumGetUInt64(res)); addHyperLogLog(&sss->abbr_card, hash); -- 2.43.0 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Refactor *_abbrev_convert() functions @ 2026-01-24 22:27 Aditya Gollamudi <[email protected]> parent: Aleksander Alekseev <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Aditya Gollamudi @ 2026-01-24 22:27 UTC (permalink / raw) To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers On Tue, Jan 13, 2026 at 4:34 AM Aleksander Alekseev < [email protected]> wrote: > Hi, > > Now when all Datums are 64-bit values we can simplify the code by > using murmurhash64(). This refactoring was previously suggested by > John Naylor [1]. > > [1]: > https://postgr.es/m/CANWCAZbMyrijdR0xc-4SqpNJBHMEwRZccBK4fa0aquNpq2Uj7w%40mail.gmail.com > > -- > Best regards, > Aleksander Alekseev > Hi, I reviewed this change and the surrounding code and this seems good! All tests pass locally for me. Hopefully this gets picked up soon. Regards, Adi Gollamudi ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Refactor *_abbrev_convert() functions @ 2026-01-29 07:25 Michael Paquier <[email protected]> parent: Aditya Gollamudi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Michael Paquier @ 2026-01-29 07:25 UTC (permalink / raw) To: Aditya Gollamudi <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers On Sat, Jan 24, 2026 at 02:27:04PM -0800, Aditya Gollamudi wrote: > I reviewed this change and the surrounding code and this seems good! > All tests pass locally for me. Hopefully this gets picked up soon. Yep, that looks rather right. Will double-check all that later. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-01-29 07:25 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-01-13 12:34 [PATCH] Refactor *_abbrev_convert() functions Aleksander Alekseev <[email protected]> 2026-01-24 22:27 ` Aditya Gollamudi <[email protected]> 2026-01-29 07:25 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox