In bug #19587[0], the user reported that hashchar() and hashcharextended() were not producing the same results for the same input because of sign extension depending on the platform.
Attached are three patches: 1. A series of tests to exercise the issue in a new TAP test and a regression test for the default path of `default_char_signedness = true`. The TAP test will show a failure. 2. A fix for the functions to ensure they produce the same results regardless of platform. 3. A consistency fix for charhashfast() such that it always produces the same value regardless of platform. This patch is kind of unimportant because the value is never persisted or sent to another process. I'll leave it up to the committer to pull the patch in or not. I am not sure that this can be backpatched before 19. I think it will cause queries to produce different values. Definitely good to fix before 19 is final though. [0]: https://www.postgresql.org/message-id/19587-4416a590531c9c73%40postgresql.org -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
From 2cee32c4a4f472851e5edca0d86568a4e533edbb Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Thu, 30 Jul 2026 06:09:43 +0000 Subject: [PATCH v1 1/3] Add tests Signed-off-by: Tristan Partin <[email protected]> --- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/015_char_signedness.pl | 64 +++++++++++++++++++ src/test/regress/expected/hash_func.out | 24 +++++++ src/test/regress/sql/hash_func.sql | 10 +++ 4 files changed, 99 insertions(+) create mode 100644 src/test/modules/test_misc/t/015_char_signedness.pl diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index ee290698b31..40e0a513bf2 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -23,6 +23,7 @@ tests += { 't/012_ddlutils.pl', 't/013_temp_obj_multisession.pl', 't/014_log_statement_max_length.pl', + 't/015_char_signedness.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/015_char_signedness.pl b/src/test/modules/test_misc/t/015_char_signedness.pl new file mode 100644 index 00000000000..d427e84419c --- /dev/null +++ b/src/test/modules/test_misc/t/015_char_signedness.pl @@ -0,0 +1,64 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that hashchar() and hashcharextended() interpret a high-bit "char" value +# according to the cluster's recorded default char signedness (see +# GetDefaultCharSignedness()), rather than the signedness the server happens to +# be compiled with. +# +# src/test/regress/sql/hash_func.sql has a companion check, but it can only ever +# exercise the "signed" branch: a freshly-initialized cluster, like the one used +# for the regular regression tests, always records "signed" (see +# WriteControlFile()). Here we use pg_resetwal to force the recorded signedness +# to "unsigned" and confirm that the SQL-level result flips accordingly, +# independent of the actual platform running the test. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('node'); +$node->init; + +# Newly-initialized clusters unconditionally record "signed". +$node->start; +is($node->safe_psql('postgres', + q{SELECT hashchar('\200'::"char") = hashint4(-128)}), + 't', + 'hashchar() on a high-bit "char" uses signed semantics when the' + . ' cluster default char signedness is signed'); +is($node->safe_psql('postgres', + q{SELECT hashcharextended('\200'::"char", 0) = hashint4extended(-128, 0)} + ), + 't', + 'hashcharextended() on a high-bit "char" uses signed semantics when the' + . ' cluster default char signedness is signed'); +$node->stop; + +# Force the recorded signedness to "unsigned" and check again. +command_ok( + [ + 'pg_resetwal', + '--char-signedness' => 'unsigned', + '--force', + $node->data_dir + ], + "set cluster's default char signedness to unsigned"); + +$node->start; +is($node->safe_psql('postgres', + q{SELECT hashchar('\200'::"char") = hashint4(128)}), + 't', + 'hashchar() on a high-bit "char" uses unsigned semantics when the' + . ' cluster default char signedness is unsigned'); +is($node->safe_psql('postgres', + q{SELECT hashcharextended('\200'::"char", 0) = hashint4extended(128, 0)} + ), + 't', + 'hashcharextended() on a high-bit "char" uses unsigned semantics when' + . ' the cluster default char signedness is unsigned'); +$node->stop; + +done_testing(); diff --git a/src/test/regress/expected/hash_func.out b/src/test/regress/expected/hash_func.out index 6bc14f57a6b..842fcf4dc6d 100644 --- a/src/test/regress/expected/hash_func.out +++ b/src/test/regress/expected/hash_func.out @@ -85,6 +85,30 @@ WHERE hashchar(v)::bit(32) != hashcharextended(v, 0)::bit(32) -------+----------+-----------+----------- (0 rows) +-- hashchar()/hashcharextended() must interpret a high-bit "char" value +-- according to the cluster's recorded default char signedness (see +-- GetDefaultCharSignedness()), not the signedness the server happens to be +-- compiled with. Newly-initialized clusters, such as the one running this +-- test, always record "signed", so a high-bit "char" must hash the same as +-- its negative signed-char-equivalent int4 value. +SELECT hashchar('\200'::"char") = hashint4(-128) AS t; + t +--- + t +(1 row) + +SELECT hashcharextended('\200'::"char", 0) = hashint4extended(-128, 0) AS t; + t +--- + t +(1 row) + +SELECT hashcharextended('\200'::"char", 1) = hashint4extended(-128, 1) AS t; + t +--- + t +(1 row) + SELECT v as value, hashname(v)::bit(32) as standard, hashnameextended(v, 0)::bit(32) as extended0, hashnameextended(v, 1)::bit(32) as extended1 diff --git a/src/test/regress/sql/hash_func.sql b/src/test/regress/sql/hash_func.sql index ce38da72c0b..ed878a40754 100644 --- a/src/test/regress/sql/hash_func.sql +++ b/src/test/regress/sql/hash_func.sql @@ -62,6 +62,16 @@ FROM (VALUES (NULL::"char"), ('1'), ('x'), ('X'), ('p'), ('N')) x(v) WHERE hashchar(v)::bit(32) != hashcharextended(v, 0)::bit(32) OR hashchar(v)::bit(32) = hashcharextended(v, 1)::bit(32); +-- hashchar()/hashcharextended() must interpret a high-bit "char" value +-- according to the cluster's recorded default char signedness (see +-- GetDefaultCharSignedness()), not the signedness the server happens to be +-- compiled with. Newly-initialized clusters, such as the one running this +-- test, always record "signed", so a high-bit "char" must hash the same as +-- its negative signed-char-equivalent int4 value. +SELECT hashchar('\200'::"char") = hashint4(-128) AS t; +SELECT hashcharextended('\200'::"char", 0) = hashint4extended(-128, 0) AS t; +SELECT hashcharextended('\200'::"char", 1) = hashint4extended(-128, 1) AS t; + SELECT v as value, hashname(v)::bit(32) as standard, hashnameextended(v, 0)::bit(32) as extended0, hashnameextended(v, 1)::bit(32) as extended1 -- Tristan Partin https://tristan.partin.io
From 0b05cf02ce504890ece0261cc61246ec548eaae3 Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Thu, 30 Jul 2026 06:18:01 +0000 Subject: [PATCH v1 2/3] Fix hashchar() and hashcharextended() to not depend on char signedness hashchar() and hashcharextended() widened their "char" argument by casting it directly to int32: return hash_uint32((int32) PG_GETARG_CHAR(0)); DatumGetChar() returns a bare char, whose signedness is implementation-defined. A high-bit byte therefore sign-extends on platforms where char defaults to signed (e.g. x86-64, macOS) but not where it defaults to unsigned (e.g. aarch64 Linux), so the two functions returned different results for the same "char" value depending on which architecture computed them: SELECT hashchar(chr(128)::"char"); x86-64: 1361043915 (= hashint4(-128)) aarch64: 1807103465 (= hashint4( 128)) Fix by widening through GetDefaultCharSignedness(), the per-cluster flag already introduced by 44fe30fdab6 to give "char"-sensitive, disk-persisting computations a stable, platform-independent notion of char signedness (as already used by pg_trgm's trigram comparators). This makes the result depend on the cluster's recorded signedness rather than the signedness the server happens to be compiled with. Fixes: https://www.postgresql.org/message-id/19587-4416a590531c9c73%40postgresql.org Signed-off-by: Tristan Partin <[email protected]> --- src/backend/access/hash/hashfunc.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/backend/access/hash/hashfunc.c b/src/backend/access/hash/hashfunc.c index 575342a21b6..245e313407b 100644 --- a/src/backend/access/hash/hashfunc.c +++ b/src/backend/access/hash/hashfunc.c @@ -26,6 +26,7 @@ #include "postgres.h" +#include "access/xlog.h" #include "common/hashfn.h" #include "utils/builtins.h" #include "utils/float.h" @@ -43,17 +44,37 @@ * routine is also used by dynahash tables. */ +/* + * Widen a "char" datum to int32 for hashing, honoring the cluster's recorded + * default char signedness (see GetDefaultCharSignedness()) rather than the + * signedness the server happens to be compiled with. + * + * A bare "char" has implementation-defined signedness, so casting it to int32 + * directly would sign-extend high-bit values on platforms where "char" defaults + * to signed (e.g. x86-64), but not on platforms where it defaults to unsigned + * (e.g. aarch64 Linux). That would make hashchar()'s result for such values + * depend on the architecture computing it, which is a problem for anything that + * persists the result, such as hash partitioning and hash indexes. See bug + * #19587. + */ +static inline int32 +char_hash_widen(const char ch) +{ + return GetDefaultCharSignedness() ? (int32) (signed char) ch : (int32) (unsigned char) ch; +} + /* Note: this is used for both "char" and boolean datatypes */ Datum hashchar(PG_FUNCTION_ARGS) { - return hash_uint32((int32) PG_GETARG_CHAR(0)); + return hash_uint32(char_hash_widen(PG_GETARG_CHAR(0))); } Datum hashcharextended(PG_FUNCTION_ARGS) { - return hash_uint32_extended((int32) PG_GETARG_CHAR(0), PG_GETARG_INT64(1)); + return hash_uint32_extended(char_hash_widen(PG_GETARG_CHAR(0)), + PG_GETARG_INT64(1)); } Datum -- Tristan Partin https://tristan.partin.io
From bf7fd6d96cb91754e0293da25a75b1774a4ad21a Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Thu, 30 Jul 2026 06:27:09 +0000 Subject: [PATCH v1 3/3] Make charhashfast() return a consistent value regardless of signedness While the output of charhashfast() is never persisted or compared across processes, it is still a good idea to make it return a consistent value regardless of the platform's implementation-defined char signedness. Signed-off-by: Tristan Partin <[email protected]> --- src/backend/utils/cache/catcache.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index 0c8955fc61a..2c3f5f7d8bf 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -196,7 +196,17 @@ chareqfast(Datum a, Datum b) static uint32 charhashfast(Datum datum) { - return murmurhash32((int32) DatumGetChar(datum)); + /* + * Cast through unsigned char, not a bare char, to avoid depending on the + * platform's implementation-defined char signedness. There's no need to + * consult GetDefaultCharSignedness() here since CatCache lives in + * backend-private memory: it's never written to disk or WAL, and every + * backend (including any physical standby) rebuilds its own copy + * independently from the catalog contents, rather than relying on a value + * computed by some other, possibly differently-built, process. Any fixed, + * deterministic choice of signedness is fine here. + */ + return murmurhash32((int32) (unsigned char) DatumGetChar(datum)); } static bool -- Tristan Partin https://tristan.partin.io
