Re: [PATCH] ternary reloption type

On 21.01.2026 22:23, Álvaro Herrera wrote:
>> 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 guess that comes from my frustration with big reloptions patch. I've
been  looking
for a way to make iy more easy to read and understand. My thought was
that if
I split them into logical "layers" showing the  development of the
patch's idea, it
would be more easy to understand them as a whole. And they were intended
to be squashed before commit, may be I should state that more clearly.

> I think it's
> strange to submit those tests in 0004, when the other half of the tests
> are for the feature in 0003.

Yeah, here you are right, I should add tests both to 0003 and 0004. I
did not think
they can be committed separately

> 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.

Second part of ternary options patch is consist of one piece, so now it
is not a problem,
and as for big reloptions patch, I think we can discuss it later. I
doubt it is readable
when it is provided in single piece.

> 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.

For ternary options with explicit "third" value, I added another error
message, it says nothing
about option type, it just lists possible values. This can be good solution.

> 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.
For the part you've committed that is correct. But with explicit "third"
option, you can't tell
for sure which value is default. Like:
prefer_XXXX_optimization: yes/no/never
It can be implemented as ternary option, but default value here can be
"yes".
I would not try to predict what behavior option developer will need, and
try to provide all
possibilities.
That's why I put default value back. But I will not be much upset if you
still decide to remove it.
Or I can remove it myself if you insist. We can add it later when
someone runs into this "never" case.
> Anyway, I have pushed the first part, after rewriting the commit
> message.

Thanks! Now the world is better place, from my point of view.

> 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.
I've reworked second part of the patch, It is in the attachment, and I
am going to create new commitfest record for it. Thank you for your work.