public inbox for [email protected]  

help / color / mirror / Atom feed

From: Álvaro Herrera <[email protected]>
To: Nikolay Shaplov <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Chris Travers <[email protected]>
Cc: Timur Magomedov <[email protected]>
Cc: Nathan Bossart <[email protected]>
Subject: Re: [PATCH] ternary reloption type
Date: Wed, 21 Jan 2026 20:23:50 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 2026-Jan-21, Nikolay Shaplov wrote:

> On 16.01.2026 18:14, Álvaro Herrera wrote:
> > I took a quick look at 0001+0002 and I think it's quite reasonable.
> > Here it is again with some minor fixups.
> 
> Good. I like ternary -> pg_ternary change. That is reasonable. And
> postgres.h is better place for it then c.h.

I further changed TERNARY_TRUE and so on to have a PG_ prefix also; it's
not impossible that there's userland code somewhere outside Postgres
that uses those symbol names, so let's avoid a collision.

> I've rebased the rest of patches, and the whole patchset is in the
> attachment.

I'm not a fan of how these commit messages are structured.  You explain
the point of the whole patch series in the commit message for 0001, but
that one only adds some tests for the existing behavior.  If I were to
commit that, it would make no sense in the overall Postgres commit
history.  So I have squashed 0001 with 0002, and also with the fraction
of 0004 that includes tests for the feature in 0002.  I think it's
strange to submit those tests in 0004, when the other half of the tests
are for the feature in 0003.  I recommend to consider how you structure
your patch splits so that they would make sense in the Postgres commit
history assuming they are committed on separate days, and that there are
multiple other commits in between.

I do agree with Nathan that there seems to be little point in the
message saying this new type is different from Boolean.  The user can
still say only "on" or "off".  Even with your "alias" proposal, there
will only be a mechanism to let the system choose between those two
values, but it will still be one or the other.  There's no provision to
have the system behave as if the user set the value to half.

Another thing I did is remove default_val for ternaries.  As far as I
can see, it makes no sense.  If your reloption defaults to either on or
off, then it's just a Boolean, right?  It can no longer be unset,
because if you unset it, then it becomes the default.

Anyway, I have pushed the first part, after rewriting the commit
message.  You can resubmit the rest after rebasing on the current tree.
I have also marked the commitfest item as committed.  Please create a
new one for the next part.

Thanks,

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"El sentido de las cosas no viene de las cosas, sino de
las inteligencias que las aplican a sus problemas diarios
en busca del progreso." (Ernesto Hernández-Novich)

view thread (3+ messages)

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], [email protected], [email protected]
  Subject: Re: [PATCH] ternary reloption type
  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