On Thu, 6 Aug 2026 at 19:55, John Naylor <[email protected]> wrote: > > On Tue, Aug 4, 2026 at 7:09 PM David Rowley <[email protected]> wrote: > > > > On Fri, 24 Jul 2026 at 06:34, John Naylor <[email protected]> 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.
The patch looks how I thought it would. > On Fri, Jul 24, 2026 at 7:04 PM Zsolt Parragi <[email protected]> > 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. Is it ever necessary to have that as a comment? Maybe it'd be better to rename ssup_datum_unsigned_cmp to ssup_datum_uint64_cmp. It just doesn't seem questionable why you'd use the 64-bit version for a 32-bit type with those names. David
