Re: BUG #16997: parameter server_encoding's category problem
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org, leiyanliang@highgo.com
Date: 2021-05-07T20:58:01Z
Lists: pgsql-bugs
Attachments
- 0001-sync-guc-code-with-docs.patch (text/x-diff) patch 0001
- 0002-remove-useless-group-categories.patch (text/x-diff) patch 0002
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes: > Thanks for reporting. All the internal parameters that can't be set by > the users are under the "Preset Options" category. Likewise, > server_encoding, lc_collate and lc_ctype should also be listed under > the "Preset Options" category, even though there is a "Client > Connection Defaults / Locale and Formatting" category, which has user > configurable parameters. For instance, since the in_hot_standby is an > internal parameter, it is specified under the "Preset Options" even > though there is a "Replication / Standby Servers" category which > mentions all the user configurable parameters. > Therefore, I would say that the documentation is correct but the code > is not. Attached patch should fix this. Hm. I did some more careful checks (by comparing the pg_settings view's output to the SGML docs) and found a boatload of additional misclassifications. The most egregious being that somebody had decided to invent a new sub-heading Write Ahead Log / Recovery, but didn't bother to add a corresponding enum value; so the variables in that sub-section were all misclassified (and not even all in the same way). 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. 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. 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. 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. * 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? > 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. regards, tom lane
Commits
-
Sync guc.c and postgresql.conf.sample with the SGML docs.
- a55a98477b69 14.0 landed