public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andrey Borodin <[email protected]>
To: Aleksander Alekseev <[email protected]>
Cc: PostgreSQL Development <[email protected]>
Cc: John Naylor <[email protected]>
Subject: Re: [PATCH] Simplify SortSupport implementation for macaddr
Date: Thu, 5 Mar 2026 23:02:45 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJ7c6TMk10rF_LiMz6j9rRy1rqk-5s+wBPuBefLix4cY+-4s1w@mail.gmail.com>
References: <CAJ7c6TMk10rF_LiMz6j9rRy1rqk-5s+wBPuBefLix4cY+-4s1w@mail.gmail.com>



> On 25 Feb 2026, at 18:05, Aleksander Alekseev <[email protected]> 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.

view thread (5+ 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]
  Subject: Re: [PATCH] Simplify SortSupport implementation for macaddr
  In-Reply-To: <[email protected]>

* 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