Re: BUG #16997: parameter server_encoding's category problem

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> On Sat, May 8, 2021 at 2:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I also discovered that postgresql.conf.sample wasn't all that well
>> in sync with the docs.  It mostly agreed with the docs as to
>> classifications, but the ordering of individual options didn't
>> agree so well.  I think this is a consequence of various people
>> having arbitrarily alphabetized the doc entries without making
>> postgresql.conf.sample match.  I can see no excuse for them not
>> to match, though.

> If the arrangement is to be in alphabetical order, then
> max_parallel_maintenance_workers should come before
> max_parallel_workers_per_gather.

No, the idea is to have postgresql.conf.sample match what's in
config.sgml, which is not at all alphabetical in that section.

We regularly have religious wars over whether strict alphabetical
order is the best way to present things.  I'm generally in the
camp that finds that to be overly pedantic, so I'm not in a hurry
to change config.sgml here.

I've pushed those patches with a bit of additional work on the
comments around enum config_group.  The other points remain
to be dealt with:

>> * I notice that these GUCs appear in the view, but if they're
>> documented anywhere it's not in config.sgml:
>> transaction_deferrable                 | Client Connection Defaults / Statement Behavior
>> transaction_isolation                  | Client Connection Defaults / Statement Behavior
>> transaction_read_only                  | Client Connection Defaults / Statement Behavior
>> Should we add entries for them?  If not, should we maybe move them
>> to the UNGROUPED category (and then make them GUC_NO_SHOW_ALL),
>> like other special GUCs such as "role"?  I'm a bit inclined to the
>> latter, since they aren't supposed to be used in the same way as
>> ordinary GUCs.

> It looks like we are using GUC_NO_SHOW_ALL for unused, deprecated, or
> the GUCs being used by other commands. Since the above 3 GUCs are
> being used by START TRANSACTION ISOLATION LEVEL command, it makes
> sense to make them GUC_NO_SHOW_ALL, but let's keep them under
> CLIENT_CONN_STATEMENT.

I think that GUCs that aren't listed in config.sgml need to be
UNGROUPED; it's just confusing to show them as part of a group
when they're not in that group according to the docs.  Also,
it's almost true that presence of GUC_NO_SHOW_ALL correlates
exactly with being UNGROUPED.  The couple of exceptions are
legacy entries that perhaps should have been changed when they
were de-documented.

>> * The code's names for the categories do not match up all that
>> well with the section headings in config.sgml, eg we have
>> Query Tuning in guc.c and Query Planning in the docs.  Should we
>> try to sync that?

> I think we can change the docs to match the guc.c code wordings. Thoughts?

Meh... not sure why we'd take the docs as gospel up to this point
and then suddenly reverse course.

After thinking about it, I think it's fine if the guc.c string is a
shortened form of the config.sgml section title.  For instance,
I don't feel a need to make "Reporting and Logging" match the docs'
"Error Reporting and Logging".  But places where it's just randomly
different, like "Query Tuning" vs. "Query Planning", probably
should be fixed to avoid confusion.

> There is another thread at [1] that discusses cleaning up the unused
> enums or modifying the GUC categories.

Ah, I'd forgotten that thread already.  I included Justin as part
author on this commit.

            regards, tom lane