| From: | Nikolay Shaplov <dhyan(at)nataraj(dot)su> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Chris Travers <chris(dot)travers(at)gmail(dot)com>, Timur Magomedov <t(dot)magomedov(at)postgrespro(dot)ru>, Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Subject: | Re: [PATCH] ternary reloption type |
| Date: | 2026-08-17 14:16:20 |
| Message-ID: | 3425885.aeNJFYEL58@thinkpad-pgpro |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
В письме от понедельник, 17 августа 2026 г. 16:03:12 Москва, стандартное время
пользователь Peter Eisentraut написал:
> > I don't like that pg_ternary was added to postgres.h.
That's understandable.
> There are, depending on how you count, a few to many other ternary types
> used throughout the tree, and it's not clear why this one should be the
> standard one now. At least if so that should have involved some
> discussion and analysis on the other ones. There are also some
> tradeoffs about how this type should be designed. This particular one
> uses 0 and 1 for false and true, and -1 for unset. Others use 0 for
> unset and other values for false and true. Maybe this choice is useful
> for this particular use, but we shouldn't impose it on everyone.
The idea was that from the reloptions point of view, ternary is a boolean with
one extra possibility. This comes about purely historically, because the
current and future ternary options are born from boolean options, so it's
convenient to keep the values that encode explicit 'yes' and 'no', so that the
corresponding fields in the database don't have to be updated when switching
from boolean to ternary. With this encoding, everything will keep working the
way it did without any pg_catalog update.
As for the 'third' value, using an enum seemed reasonable in this case, and
then you have to pick one specific value. If 0 and 1 are already taken, then -1
seems like the logical option.
When developing this patch, I wasn't aware of the existence of other ternary-
logic implementations in Postgres. I'm not against bringing these
implementations to a common style. But in the case of reloptions, we're
constrained by the fact that the data is already stored on disk and it's
better not to change it. If the other ternary values are used only in memory,
then it might be right to bring them to the same data type as the one used in
options. If you share a list of the other places where ternary logic is also
used, we'll all have a chance to look at it and assess how justified bringing
them to a common style would be.
> Independent of that, I don't understand why this was put into postgres.h
> instead of c.h. It's not particular to backend code, as far as I can tell.
> I think it would be better to rename this to something like
> relopt_ternary and move it to access/reloptions.h.
If it were up to me, I'd keep the definition of pg_ternary in access/
reloptions.h and not interfere with the core Postgres code. Unfortunately,
though, one of the ternary options value is located in the StdRdOptions struct
defined in include/utils/rel.h, so the pg_ternary type has to be defined in some
very global place. Which one exactly is debatable. In the original version of
the patch I put it in c.h. When committing, Álvaro moved it to postgres.h. I
concluded that Álvaro knows better where it should be. I don't have an opinion
of my own on this question — the main thing for me is that pg_ternary be
defined in a header file that can be included in utils/rel.h.
I guess some logic behind it might be like this: We using pg_ternary name, not
just ternary, because some other library header might also want to define
ternary. And since this type has pg_ suffix postgres.h seems to be better place
to store it, than c.h. pg_ means it is related to postgres. Things from c.h
are not postgres related.
> If we want to consolidate all ternary types, that might be useful, but
> it should be an explicit discussion.
I think we want. Me at least. Let's discuss it.
--
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Palak Chaturvedi | 2026-08-17 14:26:18 | Re: Changing shared_buffers without restart |
| Previous Message | Daniel Gustafsson | 2026-08-17 14:06:09 | Re: basebackup: do not verify checksums on pages written before enabling checksums |