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

On Sat, May 8, 2021 at 2:28 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.
#max_worker_processes = 8 # (change requires restart)
-#max_parallel_maintenance_workers = 2 # taken from max_parallel_workers
#max_parallel_workers_per_gather = 2 # taken from max_parallel_workers
-#parallel_leader_participation = on
+#max_parallel_maintenance_workers = 2 # taken from max_parallel_workers

Other changes look good to me.

> Using the same assumption that the docs represent our best ideas
> in this area, the attached 0001 makes the GUC code and the sample
> file agree with the docs.

+1 to have the code and the docs in sync.

> I also think we should do 0002, which removes the group-level
> enum values for those categories where we have sub-categories.
> The group-level enum values seem to me to just be an attractive
> bug-encouraging nuisance (there is indeed one place in 0001 that
> changes a GUC that was misclassified at the top level). Plus
> they result in translators having to do extra work.

Yeah, removing the unused group level enums CONN_AUTH, RESOURCES, WAL,
REPLICATION and so on makes sense, since all the related parameters
are categorized under respective subcategories such as CONN_AUTH_XXX,
RESOURCES_XXX, WAL_XXX, REPLICATION_XXX. If at all, any new parameters
are added and if they don't fall in any of the subcategories, a new
subcategory can be added.

> There are a couple of loose ends yet:
>
> * 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. Anyways, they will not show up in the
pg_settings and in the docs. Also, it will be good if we can add
comment before them in guc.c /* Not for general use --- used by START
TRANSACTION ISOLATION LEVEL */

> * 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?

Yeah, there are differences. (in guc.c, in docs) --> (Resource Usage,
Resource Consumption), (Write-Ahead Log, Write Ahead Log), (Query
Tuning, Query Planning), (Reporting and Logging, Error Reporting and
Logging), (Statistics, Run-time Statistics), (Autovacuum, Automatic
Vacuuming).

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

> > While on this, I also adjusted some of the wordings for the other
> > "Preset Options".
>
> I agree that the preset options should uniformly use the phrasing
> "Shows ...", but I thought your proposals here were mostly too
> verbose.

Thanks! The changes in 0001 are fine to me.

There is another thread at [1] that discusses cleaning up the unused
enums or modifying the GUC categories. And, there are some bugs being
raised for the inconsistency between what pg_settings show and what
docs say. Therefore, the patches proposed here will fix all of the
concerns.

It looks like there are more changes needed for docs clean up and for
making transaction_XXXX GUC_NO_SHOW_ALL, if agreed I can post separate
patches for these things on top of 0001 and 0002.

[1] - https://www.postgresql.org/message-id/flat/20210413123139.GE6091%40telsasoft.com

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com