Re: tuple radix sort

From: cca5507 <cca5507(at)qq(dot)com>
To: zengman <zengman(at)halodbtech(dot)com>, John Naylor <johncnaylorls(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: tuple radix sort
Date: 2026-02-11 11:46:56
Message-ID: tencent_D9FB41A25D4A685F6176ED4A4A804C06DE06@qq.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2026-02-11 11:51:55 Re: Little cleanup: Move ProcStructLock to the ProcGlobal struct
Previous Message shveta malik 2026-02-11 11:39:26 Re: [Patch] add new parameter to pg_replication_origin_session_setup