| From: | John Naylor <johncnaylorls(at)gmail(dot)com> |
|---|---|
| To: | cca5507 <cca5507(at)qq(dot)com> |
| Cc: | zengman <zengman(at)halodbtech(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: tuple radix sort |
| Date: | 2026-02-15 08:40:35 |
| Message-ID: | CANWCAZaWV09=HdiF9mm7CBggFvxGkkHhhkSqyFoochQzW=RxAA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
I've committed v7-0001 with the above review and a couple more
cosmetic adjustments. Most notably, I've never really liked the name
sort_byvalue_datum(). It really is the entry point to radix sort, and
it only diverts to qsort if after the NULL partitioning phase there
aren't enough non-null tuples to justify the overhead of radix sort.
So for better symmetry with qsort_tuple, I've renamed
sort_byvalue_datum() to radix_sort_tuple(), which then calls out to
now-named radix_sort_recursive(), which will call itself until it
completes or diverts. Another change down below:
On Fri, Feb 13, 2026 at 10:25 AM cca5507 <cca5507(at)qq(dot)com> wrote:
> I think we need to add a comment to explain why we do the
> check. The cost of this check is not small.
(presorted check) The cost should only matter for pathological inputs,
and I haven't found it to matter much overall. I've left it
uncommented for now. However, while rebasing 0002 to deal with recent
review comments, I had an idea: In yesterday's commit, I moved the
presorted check down to just before invoking the actual radix sort.
That way, with the attached v8-0002 the common prefix detection is
done at the same time as the presorted check. That makes 0002 a
smaller patch and by doing both in the same for-loop it's easier to
read and can reduce the number of memory reads. We can consider more
commentary here, but the motivation do avoid unnecessary work should
be fairly obvious.
Next step : Test whether it's worth it for the common prefix detection
to use the normalized datum.
--
John Naylor
Amazon Web Services
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0004-WIP-Call-verify_memtuples_sorted-after-qsort-for-.patch | text/x-patch | 1.5 KB |
| v8-0003-WIP-Add-possible-message-changes.patch | text/x-patch | 1.2 KB |
| v8-0002-Detect-common-prefix-to-avoid-wasted-work-during-.patch | text/x-patch | 3.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Reshmithaa | 2026-02-15 08:53:10 | Crash related to Shared Memory |
| Previous Message | Etsuro Fujita | 2026-02-15 08:40:13 | Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable |