From 8d4be376b89995a7c4ddbe872b79f7f0739e98e3 Mon Sep 17 00:00:00 2001 From: John Naylor Date: Thu, 23 Jul 2026 16:19:57 -0400 Subject: [PATCH v2] Add ssup_datum_uint32_cmp for comparing oids Commit 51cd5d6f0 used ssup_datum_unsigned_cmp for the oid comparator, which compares entire Datums. That gave wrong results, since the upper half of a Datum containing a 32-bit type is not reliably zero-extended: values fetched from tuples are sign-extended, while values returned by e.g. oidin() are zero-extended. An oid with the high bit set could therefore compare as larger or smaller depending on where it came from. Fix by adding a comparator that only looks at the low 32 bits. Also teach radix sort to normalize Datums the same way. This keeps oid eligible for radix sort. Add a regression test that sorts oids above 2^31 from both kinds of source. Reported-by: Zsolt Parragi Reviewed-by: Zsolt Parragi Suggested-by: David Rowley Discussion: https://postgr.es/m/CAN4CZFM2afRzSLJJiWKnRjqPwL7Scm2Wd1xXu140diJGuJtzGg@mail.gmail.com --- src/backend/access/nbtree/nbtcompare.c | 6 +++- src/backend/utils/sort/tuplesort.c | 39 ++++++++++++++++++------- src/include/utils/sortsupport.h | 1 + src/test/regress/expected/tuplesort.out | 11 +++++++ src/test/regress/sql/tuplesort.sql | 5 ++++ 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/backend/access/nbtree/nbtcompare.c b/src/backend/access/nbtree/nbtcompare.c index 795fded49d3..7edfa737d7a 100644 --- a/src/backend/access/nbtree/nbtcompare.c +++ b/src/backend/access/nbtree/nbtcompare.c @@ -427,7 +427,11 @@ btoidsortsupport(PG_FUNCTION_ARGS) { SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0); - ssup->comparator = ssup_datum_unsigned_cmp; + /* + * We cannot use ssup_datum_unsigned_cmp here, since the upper half of a + * Datum containing a 32-bit type is not reliably zero-extended. + */ + ssup->comparator = ssup_datum_uint32_cmp; PG_RETURN_VOID(); } diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index c0e7527b9ca..ee9c89b02d3 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -2588,24 +2588,26 @@ normalize_datum(Datum orig, SortSupport ssup) Datum norm_datum1; if (ssup->comparator == ssup_datum_signed_cmp) - { norm_datum1 = orig + (Int64GetDatum(PG_INT64_MAX)) + 1; - } - else if (ssup->comparator == ssup_datum_int32_cmp) + else if (ssup->comparator == ssup_datum_unsigned_cmp) + norm_datum1 = orig; + else { /* - * First truncate to uint32. Technically, we don't need to do this, + * Truncate to uint32. For the int32 case, we don't need to do this, * but it forces the upper half of the datum to be zero regardless of * sign. */ - uint32 u32 = DatumGetUInt32(orig) + ((uint32) PG_INT32_MAX) + 1; + uint32 u32 = DatumGetUInt32(orig); + + if (ssup->comparator == ssup_datum_int32_cmp) + norm_datum1 = UInt32GetDatum(u32 + ((uint32) PG_INT32_MAX) + 1); + else + { + norm_datum1 = UInt32GetDatum(u32); + Assert(ssup->comparator == ssup_datum_uint32_cmp); + } - norm_datum1 = UInt32GetDatum(u32); - } - else - { - Assert(ssup->comparator == ssup_datum_unsigned_cmp); - norm_datum1 = orig; } if (ssup->ssup_reverse) @@ -3011,6 +3013,7 @@ tuplesort_sort_memtuples(Tuplesortstate *state) if (state->memtupcount >= QSORT_THRESHOLD && (ssup->comparator == ssup_datum_unsigned_cmp || ssup->comparator == ssup_datum_signed_cmp || + ssup->comparator == ssup_datum_uint32_cmp || ssup->comparator == ssup_datum_int32_cmp)) { radix_sort_tuple(state->memtuples, @@ -3471,6 +3474,20 @@ ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup) return 0; } +int +ssup_datum_uint32_cmp(Datum x, Datum y, SortSupport ssup) +{ + uint32 xx = DatumGetUInt32(x); + uint32 yy = DatumGetUInt32(y); + + if (xx < yy) + return -1; + else if (xx > yy) + return 1; + else + return 0; +} + int ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup) { diff --git a/src/include/utils/sortsupport.h b/src/include/utils/sortsupport.h index a8f8f9f026a..41923f6364c 100644 --- a/src/include/utils/sortsupport.h +++ b/src/include/utils/sortsupport.h @@ -274,6 +274,7 @@ ApplySortAbbrevFullComparator(Datum datum1, bool isNull1, */ extern int ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup); extern int ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup); +extern int ssup_datum_uint32_cmp(Datum x, Datum y, SortSupport ssup); extern int ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup); /* Other functions in utils/sort/sortsupport.c */ diff --git a/src/test/regress/expected/tuplesort.out b/src/test/regress/expected/tuplesort.out index fc1321bf443..9851e8a6a57 100644 --- a/src/test/regress/expected/tuplesort.out +++ b/src/test/regress/expected/tuplesort.out @@ -703,3 +703,14 @@ EXPLAIN (COSTS OFF) :qry; (10 rows) COMMIT; +-- Test sorting oids with the high bit set, mixing sources +CREATE TEMP TABLE test_oid_sort (o oid); +INSERT INTO test_oid_sort VALUES ('2147483648'), ('2147483647'); +SELECT o FROM test_oid_sort UNION ALL SELECT '3000000000'::oid ORDER BY 1; + o +------------ + 2147483647 + 2147483648 + 3000000000 +(3 rows) + diff --git a/src/test/regress/sql/tuplesort.sql b/src/test/regress/sql/tuplesort.sql index 8476e594e6c..31cd28c6116 100644 --- a/src/test/regress/sql/tuplesort.sql +++ b/src/test/regress/sql/tuplesort.sql @@ -305,3 +305,8 @@ EXPLAIN (COSTS OFF) :qry; :qry; COMMIT; + +-- Test sorting oids with the high bit set, mixing sources +CREATE TEMP TABLE test_oid_sort (o oid); +INSERT INTO test_oid_sort VALUES ('2147483648'), ('2147483647'); +SELECT o FROM test_oid_sort UNION ALL SELECT '3000000000'::oid ORDER BY 1; -- 2.55.0