Re: tuple radix sort
cca5507 <cca5507@qq.com>
From: cca5507 <cca5507@qq.com>
To: zengman <zengman@halodbtech.com>, John Naylor <johncnaylorls@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-02-11T11:46:56Z
Lists: pgsql-hackers
Hi,
> I'm wondering if we should replace `state->memtuples` with `data` in the `sort_byvalue_datum()` function here.
+1. Now data == state->memtuples, how about remove the parameter "data" and "n" and just
get them from "Tuplesortstate"?
Some comments for v7:
1)
```
/*
* Retrieve byte from datum, indexed by 'level': 0 for LSB, 7 for MSB
*/
static inline uint8
current_byte(Datum key, int level)
{
int shift = (SIZEOF_DATUM - 1 - level) * BITS_PER_BYTE;
return (key >> shift) & 0xFF;
}
```
Maybe "0 for MSB, 7 for LSB"? If level == 0, this function will return the Most Significant Byte.
2) radix_sort_tuple()
```
size_t end_offset = partitions[*rp].next_offset;
SortTuple *partition_end = begin + end_offset;
ptrdiff_t num_elements = end_offset - start_offset;
```
Why the type of "num_elements" is "ptrdiff_t"? Maybe just "size_t"?
3) tuplesort_sort_memtuples()
```
/*
* Do we have the leading column's value or abbreviation in datum1?
*/
if (state->base.haveDatum1 && state->base.sortKeys)
{
SortSupportData ssup = state->base.sortKeys[0];
```
I think we should avoid the copy of SortSupportData. We can just use a pointer.
4)
Many places just consider "Datum" as integer, do we need to add a DatumGetUInt**() for them?
--
Regards,
ChangAo Chen
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Cosmetic fixes for radix sort
- ca9807dfec5b 19 (unreleased) landed
-
Skip common prefixes during radix sort
- f6bd9f0fe25a 19 (unreleased) landed
-
Perform radix sort on SortTuples with pass-by-value Datums
- ef3c3cf6d021 19 (unreleased) landed
-
Grab the low-hanging fruit from forcing sizeof(Datum) to 8.
- 6aebedc38497 19 (unreleased) cited
-
Add missing Datum conversions
- ff89e182d420 19 (unreleased) cited