Thread: Docs: Always use true|false instead of sometimes on|off for the subscription options

Hi hackers,

There are lots of subscription options listed on the CREATE
SUBSCRIPTION page [1].

Although these boolean options are capable of accepting different
values like "1|0", "on|off", "true|false", here they are all described
only using values "true|false".

~

IMO this consistent way of describing the boolean option values ought
to be followed also on the ALTER SUBSCRIPTION page [2]. Specifically,
the ALTER SUBSCRIPTION page has one mention of "SET (failover =
on|off)" which I think should be changed to say "SET (failover =
true|false)"

Now this little change hardly seems important, but actually, it is
motivated by another thread [3] under development where this ALTER
SUBSCRIPTION "(failover = on|off)" was copied again, thereby making
the consistency between the CREATE SUBSCRIPTION and ALTER SUBSCRIPTION
pages grow further apart, so I think it is best to nip that in the bud
and simply use "true|false" values everywhere.

PSA a patch to make this proposed change.

======
[1] https://www.postgresql.org/docs/devel/sql-createsubscription.html
[2] https://www.postgresql.org/docs/devel/sql-altersubscription.html
[3] https://www.postgresql.org/message-id/flat/8fab8-65d74c80-1-2f28e880%4039088166

Kind Regards,
Peter Smith.
Fujitsu Australia

On Thu, 16 May 2024 at 12:29, Peter Smith <smithpb2250@gmail.com> wrote:
> There are lots of subscription options listed on the CREATE
> SUBSCRIPTION page [1].
>
> Although these boolean options are capable of accepting different
> values like "1|0", "on|off", "true|false", here they are all described
> only using values "true|false".

If you want to do this, what's the reason to limit it to just this one
page of the docs?

If the following is anything to go by, it doesn't seem we're very
consistent about this over the entire documentation.

doc$ git grep "<literal>on</literal>" | wc -l
122

doc$ git grep "<literal>true</literal>" | wc -l
222

And:

doc$ git grep "<literal>off</literal>" | wc -l
102

doc$ git grep "<literal>false</literal>" | wc -l
162

I think unless we're going to standardise on something then there's
not much point in adjusting individual cases. IMO, there could be an
endless stream of follow-on patches as a result of accepting this.

David

On Thu, May 16, 2024 at 3:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 16 May 2024 at 12:29, Peter Smith <smithpb2250@gmail.com> wrote:
> > There are lots of subscription options listed on the CREATE
> > SUBSCRIPTION page [1].
> >
> > Although these boolean options are capable of accepting different
> > values like "1|0", "on|off", "true|false", here they are all described
> > only using values "true|false".
>
> If you want to do this, what's the reason to limit it to just this one
> page of the docs?

Yeah, I had a vested interest in this one place because I've been
reviewing the other thread [1] that was mentioned above. If that other
thread chooses "true|false" then putting "true|false" adjacent to
another "on|off" will look silly. But if that other thread adopts the
existing 'failover=on|off' values then it will diverge even further
from being consistent with the CREATE SUBSCRIPTION page.
Unfortunately, that other thread cannot take it upon itself to make
this change because it has nothing to do with the 'failover' option,
So I saw no choice but to post an independent patch for this.

I checked all the PUBLICATION/SUBSCRIPTION reference pages and this
was the only inconsistent value that I found. But I might be mistaken.

>
> If the following is anything to go by, it doesn't seem we're very
> consistent about this over the entire documentation.
>
> doc$ git grep "<literal>on</literal>" | wc -l
> 122
>
> doc$ git grep "<literal>true</literal>" | wc -l
> 222
>
> And:
>
> doc$ git grep "<literal>off</literal>" | wc -l
> 102
>
> doc$ git grep "<literal>false</literal>" | wc -l
> 162
>

Hmm. I'm not entirely sure if those stats are meaningful because I'm
not saying option values should avoid "on|off" -- the point was I just
suggesting they should be used consistent with where they are
described. For example, the CREATE SUBSCRIPTION page describes every
boolean option value as "true|false", so let's use "true|false" in
every other docs place where those are mentioned. OTOH, other options
on other pages may be described as "on|off" which is fine by me, but
then those should similarly be using "on|off" again wherever they are
mentioned.

> I think unless we're going to standardise on something then there's
> not much point in adjusting individual cases. IMO, there could be an
> endless stream of follow-on patches as a result of accepting this.
>

Standardisation might be ideal, but certainly, I'm not going to
attempt to make a giant patch that impacts the entire documentation
just for this one small improvement.

It seems a shame if "perfect" becomes the enemy of "good"; How else do
you suggest I can make the ALTER SUBSCRIPTION page better? If this
one-line change is rejected then the most likely outcome is nothing
will ever happen to change it.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Thu, 16 May 2024 at 19:05, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Thu, May 16, 2024 at 3:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
> > If you want to do this, what's the reason to limit it to just this one
> > page of the docs?
>
> Yeah, I had a vested interest in this one place because I've been
> reviewing the other thread [1] that was mentioned above. If that other
> thread chooses "true|false" then putting "true|false" adjacent to
> another "on|off" will look silly.

OK, looking a bit further I see this option is new to v17.  After a
bit more study of the sgml, I agree that it's worth changing this.

I've pushed your patch.

David

Peter Smith <smithpb2250@gmail.com> writes:
> Yeah, I had a vested interest in this one place because I've been
> reviewing the other thread [1] that was mentioned above. If that other
> thread chooses "true|false" then putting "true|false" adjacent to
> another "on|off" will look silly. But if that other thread adopts the
> existing 'failover=on|off' values then it will diverge even further
> from being consistent with the CREATE SUBSCRIPTION page.

It's intentional that we allow more than one spelling of boolean
values.  I can see some value in being consistent within nearby
examples, but I don't agree at all that it should be uniform
across all our docs.

            regards, tom lane

On Thu, May 16, 2024 at 10:42 PM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 16 May 2024 at 19:05, Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Thu, May 16, 2024 at 3:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
> > > If you want to do this, what's the reason to limit it to just this one
> > > page of the docs?
> >
> > Yeah, I had a vested interest in this one place because I've been
> > reviewing the other thread [1] that was mentioned above. If that other
> > thread chooses "true|false" then putting "true|false" adjacent to
> > another "on|off" will look silly.
>
> OK, looking a bit further I see this option is new to v17.  After a
> bit more study of the sgml, I agree that it's worth changing this.
>
> I've pushed your patch.
>

Thanks very much for pushing. It was just a one-time thing -- I won't
go looking for more examples like it.

======
Kind Regards,
Peter Smith.
Fujitsu Australia