| From: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
|---|---|
| To: | Aleksander Alekseev <aleksander(at)tigerdata(dot)com> |
| Cc: | PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>, John Naylor <johncnaylorls(at)gmail(dot)com> |
| Subject: | Re: [PATCH] Simplify SortSupport implementation for macaddr |
| Date: | 2026-03-05 18:02:45 |
| Message-ID: | 932CBCA0-FF4B-406E-9296-1B8A63C3EB49@yandex-team.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 25 Feb 2026, at 18:05, Aleksander Alekseev <aleksander(at)tigerdata(dot)com> wrote:
>
> <v1-0001-Simplify-SortSupport-implementation-for-macaddr.patch>
The patch looks correct and useful to me.
Two small points:
1. The assignment ssup->ssup_extra = NULL can be removed. The
SortSupport struct is zeroed before the callback is called (see
sortsupport.h). There are about 22 similar assignments elsewhere;
it does not seem to be established practice, many other places have
no such assignment.
2. I checked whether the existing tests actually use the SortSupport
path. If macaddr_abbrev_abort() is made to error out, the macaddr.sql
tests fail in two places: once for the index and once for the SELECT.
So the current test file does exercise the SortSupport code path. I
also tried making macaddr_abbrev_abort() always return true (so
abbreviation is always aborted); the test dataset is small, but
sorting seem to produce correct results.
The patch looks good to me.
Best regards, Andrey Borodin.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joe Conway | 2026-03-05 18:06:37 | Re: Emitting JSON to file using COPY TO |
| Previous Message | g l | 2026-03-05 17:34:21 | modifying join_clause_is_movable_to can produce better query plan |