On Sat, Jan 29, 2022 at 06:18:50PM -0600, Justin Pryzby wrote: > "The most meaningful ones are included" doesn't seem to add anything. > Maybe it'd be useful to say "(Only the most useful flags are exposed)"
Yes, I have used something like that. > I think the description is wrong, or just copied from the others. > EXPLAIN is for GUCs which are shown in EXPLAIN(SETTINGS). Added some details here. > |EXPLAIN, parameters included in EXPLAIN commands. > |NO_SHOW_ALL, parameters excluded from SHOW ALL commands. > |NO_RESET_ALL, parameters excluded from RESET ALL commands. > |NOT_IN_SAMPLE, parameters not included in postgresql.conf by default. > |RUNTIME_COMPUTED, runtime-computed parameters. > > Instead of a comma, these should use a colon, or something else? And switched to a colon here. With all those doc fixes, applied after an extra round of review. So this makes us rather covered with the checks on the flags. Now, what do we do with the rest of check_guc that involve a direct lookup at what's on disk. We have the following: 1) Check the format of the option lists in guc.c. 2) Check the format of postgresql.conf.sample: -- Valid options preceded by a '#' character. -- Valid options followed by ' =', with at least one space before the equal sign. 3) Check that options not marked as NOT_IN_SAMPLE are in the sample file. I have never seen 1) as a problem, and pgindent takes care of that at some degree. 2) is also mostly cosmetic, and committers are usually careful when adding a new GUC. 3) would be the most interesting piece, and would cover most cases if we consider that a default installation just copies postgresql.conf.sample over, as proposed upthread in 0002. Now, 3) has also the problem that it would fail installcheck as one can freely add a developer option in the configuration. We could solve that by adding a check in a TAP test, by using pg_config --sharedir to find where the sample file is located. I wonder if this would be a problem for some distributions, though, so adding such a dependency feels a bit scary even if it would mean that initdb is patched. As a whole, I'd like to think that we would not lose much if check_guc is removed. -- Michael
signature.asc
Description: PGP signature
