Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: [PATCH] Simplify SortSupport implementation for macaddr
From: Andrey Borodin <x4mmm@yandex-team.ru>
In-Reply-To: 
 <CAJ7c6TMk10rF_LiMz6j9rRy1rqk-5s+wBPuBefLix4cY+-4s1w@mail.gmail.com>
Date: Thu, 5 Mar 2026 23:02:45 +0500
Cc: PostgreSQL Development <pgsql-hackers@postgresql.org>,
 John Naylor <johncnaylorls@gmail.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <932CBCA0-FF4B-406E-9296-1B8A63C3EB49@yandex-team.ru>
References: 
 <CAJ7c6TMk10rF_LiMz6j9rRy1rqk-5s+wBPuBefLix4cY+-4s1w@mail.gmail.com>
To: Aleksander Alekseev <aleksander@tigerdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/932CBCA0-FF4B-406E-9296-1B8A63C3EB49%40yandex-team.ru>
Precedence: bulk


> On 25 Feb 2026, at 18:05, Aleksander Alekseev =
<aleksander@tigerdata.com> wrote:
>=20
> <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 =3D 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.=