public inbox for [email protected]  

help / color / mirror / Atom feed

From: John Naylor <[email protected]>
To: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Peter Geoghegan <[email protected]>
Subject: Re: tuple radix sort
Date: Thu, 30 Oct 2025 10:40:44 +0700
Message-ID: <CANWCAZbrKH-sDvzqb5z8BEofpXyXuN42UCRzoQ8wT3CiqPATFg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CANWCAZYzx7a7E9AY16Jt_U3+GVKDADfgApZ-42SYNiig8dTnFA@mail.gmail.com>
	<[email protected]>

On Thu, Oct 30, 2025 at 8:56 AM Chao Li <[email protected]> wrote:

> I changed work_men to 1GB and reran the test. As the high cardinality data are still there, so I first reran with data:

> With radix_sort on and off, execution time are almost the same.

Are you by chance running with asserts on? It's happened before, so I
have to make sure. That makes a big difference here because I disabled
diversion thresholds in assert builds so that regression tests (few
cases with large inputs) cover the paths I want, in addition to my
running a standalone stress test.

Speaking of tests, I forgot to mention that regression tests will fail
since in-place radix sort is an unstable sort, as qsort is as well,
but in a different way -- this is expected. In assert builds, the
patch verifies the sort after the fact with the standard comparator,
and will throw an error if it's wrong.

On Thu, Oct 30, 2025 at 9:19 AM Chao Li <[email protected]> wrote:
> I just quick went through the code change. I guess I need more time to understand the entire logic, but I find a typo that might effect the tests:
>
> ```
> +       Assert(last = first);
> ```
>
> “=“ should be “=="

Yes, you're quite right, thanks for spotting! I reran my stress test
that has randomly distributed NULLs and the assert still holds, so
nothing further to fix yet. The NULL partitioning part of the code
hasn't been well tested in its current form, and I may arrange things
so that that step and the datum conditioning step happen in a single
pass. I'm not yet sure if it's important enough to justify the
additional complexity.

--
John Naylor
Amazon Web Services

view thread (39+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: tuple radix sort
  In-Reply-To: <CANWCAZbrKH-sDvzqb5z8BEofpXyXuN42UCRzoQ8wT3CiqPATFg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox