Re: [PATCH] Use ssup_datum_*_cmp for int2, oid, and oid8 sort support

On Tue, Aug 4, 2026 at 7:09 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Fri, 24 Jul 2026 at 06:34, John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
> > I think the easiest fix is to revert the oid part of commit 51cd5d6f0,
> > leaving behind the int2 and oid8 parts. The asymmetry between the 2
> > oid types would look odd, though, so that would require an explanatory
> > comment.
>
> I didn't see it mentioned, but just for the archives' sake, did you
> rule out adding a dedicated uint32 comparator function?
>
> Or is there some other reason this can't be done due to the radix sort code?

To be honest, I hadn't put much thought into it, but it seems like a
good invariant to keep that all integer types with normal comparison
semantics are eligible for radix sort. v2 goes in this direction, and
I've run the same tests used when developing radix sort.

On Fri, Jul 24, 2026 at 7:04 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
> + * We cannot use ssup_datum_unsigned_cmp here, since we cannot count on
> + * Datums being zero-extended.
>
> One nitpick that this explains the why, but it doesn't mention the
> difference with oid8.

How about:

+ /*
+ * 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.
+ */

...by mentioning 32-bit the difference from oid8 should be obvious, I hope.

--
John Naylor
Amazon Web Services