public inbox for [email protected]help / color / mirror / Atom feed
[PATCH] Use ssup_datum_*_cmp for int2, oid, and oid8 sort support 3+ messages / 2 participants [nested] [flat]
* [PATCH] Use ssup_datum_*_cmp for int2, oid, and oid8 sort support @ 2026-06-03 23:37 Baji Shaik <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Baji Shaik @ 2026-06-03 23:37 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected] Hi, While auditing nbtcompare.c, I noticed that int2, oid, and oid8 sortsupport functions use custom local fastcmp helpers that are functionally equivalent to the existing ssup_datum_int32_cmp (for int2) and ssup_datum_unsigned_cmp (for oid, oid8). This prevents these types from hitting the radix sort fast path added in commit ef3c3cf6d02 [1], which dispatches based on the comparator function pointer. The original 2021-2022 thread that introduced the ssup_datum_*_cmp helpers (commit 6974924347c, Apr 2022) [2] covered int4, int8, timestamp, date, and the abbreviated-key types (text, uuid, macaddr, inet, bytea). int2 and oid weren't called out in that discussion, and oid8 didn't exist at the time, it was added in Jan 2026 [3] and inherited the custom fastcmp pattern from oid, about a month before radix sort landed. This patch fills those gaps. Switching to the existing helpers makes these types eligible for radix sort. Benchmark (10M-row single-key sort): Type Before After Speedup ---- ------ ----- ------- int2 2440 ms 1778 ms ~27% oid 2875 ms 2073 ms ~28% oid8 2837 ms 2042 ms ~28% int4 -- 1765 ms (baseline) int8 -- 2031 ms (baseline) The patch just replaces the comparator assignment and removes the now-unused local fastcmp functions. No behavioral change and the helpers produce identical results. int2 uses ssup_datum_int32_cmp because there is no int16-specific helper, every int16 fits losslessly in int32, and int32_cmp is more efficient than signed_cmp (4-byte radix passes instead of 8). Other custom fastcmp users in core (float4/float8, varlena types) cannot be trivially switched due to NaN handling or locale-dependent comparison, so they are left as-is. Tested with make check (245/245 pass) and make isolation/check (128/128 pass). [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ef3c3cf6d02 [2] https://www.postgresql.org/message-id/flat/CA%2BhUKGJ2-eaDqAum5bxhpMNhvuJmRDZxB_Tow0n-gse%2BHG0Yig%4... [3] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=b139bd3b6 Thanks, Baji Shaik Attachments: [application/octet-stream] 0001-Use-ssup_datum_-_cmp-for-int2-oid-and-oid8-sort-supp.patch (2.6K, 3-0001-Use-ssup_datum_-_cmp-for-int2-oid-and-oid8-sort-supp.patch) download | inline diff: From e23fc1c9a28435f56cbb24e6fc23c85dde4062da Mon Sep 17 00:00:00 2001 From: Baji Shaik <[email protected]> Date: Wed, 27 May 2026 01:48:05 +0000 Subject: [PATCH] Use ssup_datum_*_cmp for int2, oid and oid8 sort support The int2, oid, and oid8 sortsupport functions used custom fastcmp helpers that are functionally equivalent to ssup_datum_int32_cmp (for int2) and ssup_datum_unsigned_cmp (for oid, oid8). This prevented these types from using the radix sort fast path added in commit ef3c3cf6d02, which dispatches based on the comparator pointer. Switching to the existing helpers makes int2, oid, and oid8 column sorts eligible for radix sort, bringing them to parity with int4/int8 baseline. Author: Baji Shaik <[email protected]> Reviewed-by: Discussion: --- src/backend/access/nbtree/nbtcompare.c | 43 ++------------------------ 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/src/backend/access/nbtree/nbtcompare.c b/src/backend/access/nbtree/nbtcompare.c index 1d343377e98..795fded49d3 100644 --- a/src/backend/access/nbtree/nbtcompare.c +++ b/src/backend/access/nbtree/nbtcompare.c @@ -134,21 +134,12 @@ btint2cmp(PG_FUNCTION_ARGS) PG_RETURN_INT32((int32) a - (int32) b); } -static int -btint2fastcmp(Datum x, Datum y, SortSupport ssup) -{ - int16 a = DatumGetInt16(x); - int16 b = DatumGetInt16(y); - - return (int) a - (int) b; -} - Datum btint2sortsupport(PG_FUNCTION_ARGS) { SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0); - ssup->comparator = btint2fastcmp; + ssup->comparator = ssup_datum_int32_cmp; PG_RETURN_VOID(); } @@ -431,26 +422,12 @@ btoidcmp(PG_FUNCTION_ARGS) PG_RETURN_INT32(A_LESS_THAN_B); } -static int -btoidfastcmp(Datum x, Datum y, SortSupport ssup) -{ - Oid a = DatumGetObjectId(x); - Oid b = DatumGetObjectId(y); - - if (a > b) - return A_GREATER_THAN_B; - else if (a == b) - return 0; - else - return A_LESS_THAN_B; -} - Datum btoidsortsupport(PG_FUNCTION_ARGS) { SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0); - ssup->comparator = btoidfastcmp; + ssup->comparator = ssup_datum_unsigned_cmp; PG_RETURN_VOID(); } @@ -513,26 +490,12 @@ btoid8cmp(PG_FUNCTION_ARGS) PG_RETURN_INT32(A_LESS_THAN_B); } -static int -btoid8fastcmp(Datum x, Datum y, SortSupport ssup) -{ - Oid8 a = DatumGetObjectId8(x); - Oid8 b = DatumGetObjectId8(y); - - if (a > b) - return A_GREATER_THAN_B; - else if (a == b) - return 0; - else - return A_LESS_THAN_B; -} - Datum btoid8sortsupport(PG_FUNCTION_ARGS) { SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0); - ssup->comparator = btoid8fastcmp; + ssup->comparator = ssup_datum_unsigned_cmp; PG_RETURN_VOID(); } -- 2.50.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Use ssup_datum_*_cmp for int2, oid, and oid8 sort support @ 2026-06-05 03:45 Michael Paquier <[email protected]> parent: Baji Shaik <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Michael Paquier @ 2026-06-05 03:45 UTC (permalink / raw) To: Baji Shaik <[email protected]>; +Cc: [email protected]; [email protected] On Wed, Jun 03, 2026 at 06:37:09PM -0500, Baji Shaik wrote: > int2 uses ssup_datum_int32_cmp because there is no int16-specific > helper, every int16 fits losslessly in int32, and int32_cmp is more > efficient than signed_cmp (4-byte radix passes instead of 8). That's nice for such a simple change. That seems correct to me. Could you add that to the next commit fest please at [1]? > Other custom fastcmp users in core (float4/float8, varlena types) > cannot be trivially switched due to NaN handling or locale-dependent > comparison, so they are left as-is. Nope, we cannot do that. [1]: https://commitfest.postgresql.org/59/ -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Use ssup_datum_*_cmp for int2, oid, and oid8 sort support @ 2026-06-05 18:35 Baji Shaik <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Baji Shaik @ 2026-06-05 18:35 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: [email protected]; [email protected] On Thu, Jun 4, 2026 at 10:45 PM Michael Paquier <[email protected]> wrote: > That's nice for such a simple change. That seems correct to me. > Could you add that to the next commit fest please at [1]? > Thanks for the review. Added to the commitfest: https://commitfest.postgresql.org/patch/6851/ ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-06-05 18:35 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-06-03 23:37 [PATCH] Use ssup_datum_*_cmp for int2, oid, and oid8 sort support Baji Shaik <[email protected]> 2026-06-05 03:45 ` Michael Paquier <[email protected]> 2026-06-05 18:35 ` Baji Shaik <[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