Thread: GUC flags
This isn't flagged with GUC_EXPLAIN:
enable_incremental_sort
Here's some more candidates:
shared_buffers - it seems unfortunate this is not included; actually, it seems
like maybe this should be always included - not just if it's set to a
non-default, but especially if it's left at the default..
I suppose it's more important for DML than SELECT.
temp_tablespaces isn't, but temp_buffers is;
autovacuum - if it's off, that can be the cause of the issue (same as force_parallel_mode, which has GUC_EXPLAIN);
max_worker_processes - isn't, but these are: max_parallel_workers, max_parallel_workers_per_gather;
track_io_timing - it can have high overhead;
session_preload_libraries, shared_preload_libraries, local_preload_libraries;
debug_assertions - it's be kind of nice to show this whenever it's true (even thought it's not "changed")
debug_discard_caches
lc_collate and lc_ctype ?
server_version_num - I asked about this in the past, but Tom thought it should
not be included, and I kind of agree, but I'm including it here for
completeness sake
This isn't marked GUC_NOT_IN_SAMPLE, like all other DEVELOPER_OPTIONS:
trace_recovery_messages
I'm not sure jit_tuple_deforming should be marked GUC_NOT_IN_SAMPLE.
I disable this one because it's slow for tables with many attributes.
Same for jit_expressions ?
bgwriter_lru_maxpages should have GUC_UNIT_BLOCKS
max_identifier_length should have BYTES (like log_parameter_max_length and
track_activity_query_size).
block_size and wal_block_size should say BYTES (like wal_segment_size)
And all three descriptions should say ".. in [prefix]bytes" (but see below).
Maybe these should have COMPAT_OPTIONS_PREVIOUS:
integer_datetimes
ssl_renegotiation_limit
autovacuum_freeze_max_age has a comment about pg_resetwal which is obsolete
since 74cf7d46a.
checkpoint_warning refers to "checkpoint segments", which is obsolete since
88e982302.
The attached does the least-disputable, lowest hanging fruit.
More ideas:
Maybe maintenance_io_concurrency should not be GUC_EXPLAIN. But it's used not
only by ANALYZE but also heap_index_delete_tuples.
Should these be GUC_RUNTIME_COMPUTED?
in_hot_standby, data_directory_mode
Since GUC_DISALLOW_IN_FILE effectively implies GUC_NOT_IN_SAMPLE in
src/backend/utils/misc/help_config.c:displayStruct(), many of the redundant
GUC_NOT_IN_SAMPLE could be removed.
I think several things with COMPAT_OPTIONS_PREVIOUS could have
GUC_NO_SHOW_ALL and/or GUC_NOT_IN_SAMPLE.
The GUC descriptions are a hodge podge of full sentences and telegrams.
There's no consistency whether the long description can be read independently
from the short description. For these GUCs, the short description reads more
like a "DETAIL" message:
|trace_recovery_messages, log_min_error_statement, log_min_messages, client_min_messages
|log_transaction_sample_rate, log_statement_sample_rate
|data_directory_mode, log_file_mode, unix_socket_permissions
|log_directory, log_destination, log_line_prefix
|unix_socket_group, default_tablespace, DateStyle, maintenance_work_mem, geqo_generations
For integer/real GUCs, the long description is being used just to describe the
"special" values:
|jit_inline_above_cost, jit_optimize_above_cost, jit_above_cost, log_startup_progress_interval,
|tcp_user_timeout, tcp_keepalives_interval, tcp_keepalives_idle, log_temp_files, old_snapshot_threshold,
|log_parameter_max_length_on_error, log_parameter_max_length, log_autovacuum_min_duration, log_min_duration_sample,
|idle_session_timeout, idle_in_transaction_session_timeout, lock_timeout,
|statement_timeout, shared_memory_size_in_huge_pages
Descriptions of some GUCs describe their default units, but other's don't.
The units are not very important for input, since a non-default unit can be
specified, like SET statement_timeout='1h'. It's important for output, and
SHOW already includes a unit, which may not be the default unit. So I think
the default units should be removed from the descriptions.
This cleanup is similar to GUC categories fixed in a55a98477.
Tom was of the impression that there's more loose ends on that thread.
https://www.postgresql.org/message-id/flat/16997-ff16127f6e0d1390@postgresql.org
--
Justin
Attachment
On Sun, Nov 28, 2021 at 09:08:33PM -0600, Justin Pryzby wrote:
> This isn't flagged with GUC_EXPLAIN:
> enable_incremental_sort
Yeah, that's inconsistent.
> This isn't marked GUC_NOT_IN_SAMPLE, like all other DEVELOPER_OPTIONS:
> trace_recovery_messages
Indeed.
> I'm not sure jit_tuple_deforming should be marked GUC_NOT_IN_SAMPLE.
> I disable this one because it's slow for tables with many attributes.
> Same for jit_expressions ?
That would be consistent. Both are not in postgresql.conf.sample.
> bgwriter_lru_maxpages should have GUC_UNIT_BLOCKS
>
> max_identifier_length should have BYTES (like log_parameter_max_length and
> track_activity_query_size).
>
> block_size and wal_block_size should say BYTES (like wal_segment_size)
> And all three descriptions should say ".. in [prefix]bytes" (but see below).
Okay for these.
> Maybe these should have COMPAT_OPTIONS_PREVIOUS:
> integer_datetimes
> ssl_renegotiation_limit
Hmm. Okay as well for integer_datetimes.
> autovacuum_freeze_max_age has a comment about pg_resetwal which is obsolete
> since 74cf7d46a.
>
> checkpoint_warning refers to "checkpoint segments", which is obsolete since
> 88e982302.
That's part of 0002. That's a bit weird to use now, so I'd agree with
your suggestion to use "WAL segments" instead.
0001, to adjust the units, and 0003, to make the GUC descriptions less
unit-dependent, are good ideas.
- gettext_noop("Use of huge pages on Linux or Windows."),
+ gettext_noop("Enable use of huge pages on Linux or Windows."),
This should be "Enables use of".
{"compute_query_id", PGC_SUSET, STATS_MONITORING,
- gettext_noop("Compute query identifiers."),
+ gettext_noop("Enables in-core computation of a query identifier."),
This could just be "Computes"?
I am not completely sure that all the contents of 0002 are
improvements, but the suggestions done for huge_pages,
ssl_passphrase_command_supports_reload, checkpoint_warning and
commit_siblings seem fine to me.
--
Michael
Attachment
On Mon, Nov 29, 2021 at 05:04:01PM +0900, Michael Paquier wrote: > 0001, to adjust the units, and 0003, to make the GUC descriptions less > unit-dependent, are good ideas. Actually, after sleeping on it and doing some digging in codesearch.debian.org, changing the units of max_identifier_length, block_size and wal_block_size could induce some breakages for anything using a SHOW command, something that becomes particularly worse now that SHOW is supported in replication connections, and it would force clients to know and parse the units of a value. Perhaps I am being too careful here, but that could harm a lot of users. It is worth noting that I have found some driver code making use of pg_settings, which would not be influenced by such a change, but it is unsafe to assume that everybody does that. The addition of GUC_EXPLAIN for enable_incremental_sort, the comment fix for autovacuum_freeze_max_age, the use of COMPAT_OPTIONS_PREVIOUS for ssl_renegotiation_limit and the addition of GUC_NOT_IN_SAMPLE for trace_recovery_messages are fine, though. > I am not completely sure that all the contents of 0002 are > improvements, but the suggestions done for huge_pages, > ssl_passphrase_command_supports_reload, checkpoint_warning and > commit_siblings seem fine to me. Hmm, I think the patched description of checkpoint_warning is not that much an improvement compared to the current one. While the current description uses the term "checkpoint segments", which is, I agree, weird. The new one would lose the term "checkpoint", making the short description of the parameter lose some of its context. I have done a full review of the patch set, and applied the obvious fixes/improvements as of be54551. Attached is an extra patch based on the contents of the whole set sent upthread: - Improvement of the description of checkpoint_segments. - Reworded the description of all parameters using "N units", rather than just switching to "this period of units". I have been using something more generic. Thoughts? -- Michael
Attachment
On Tue, Nov 30, 2021 at 03:36:45PM +0900, Michael Paquier wrote:
> - gettext_noop("Forces a switch to the next WAL file if a "
> - "new file has not been started within N seconds."),
> + gettext_noop("Sets the amount of time to wait before forcing a "
> + "switch to the next WAL file."),
..
> - gettext_noop("Waits N seconds on connection startup after authentication."),
> + gettext_noop("Sets the amount of seconds to wait on connection "
> + "startup after authentication."),
"amount of time", like above.
> - gettext_noop("Waits N seconds on connection startup before authentication."),
> + gettext_noop("Sets the amount of seconds to wait on connection "
> + "startup before authentication."),
same
> {
> {"checkpoint_warning", PGC_SIGHUP, WAL_CHECKPOINTS,
> - gettext_noop("Enables warnings if checkpoint segments are filled more "
> - "frequently than this."),
> + gettext_noop("Sets the maximum time before warning if checkpoints "
> + "triggered by WAL volume happen too frequently."),
> gettext_noop("Write a message to the server log if checkpoints "
> - "caused by the filling of checkpoint segment files happens more "
> + "caused by the filling of WAL segment files happens more "
It should say "happen" , since it's referring to "checkpoints".
That was a pre-existing issue.
> {"log_parameter_max_length", PGC_SUSET, LOGGING_WHAT,
> - gettext_noop("When logging statements, limit logged parameter values to first N bytes."),
> + gettext_noop("Sets the maximum amount of data logged for bind "
> + "parameter values when logging statements."),
I think this one should actually say "in bytes" or at least say "maximum
length". It seems unlikely that someone is going to specify this in other
units, and it's confusing to everyone else to refer to "amount of data" instead
of "length in bytes".
> {"log_parameter_max_length_on_error", PGC_USERSET, LOGGING_WHAT,
> - gettext_noop("When reporting an error, limit logged parameter values to first N bytes."),
> + gettext_noop("Sets the maximum amount of data logged for bind "
> + "parameter values when logging statements, on error."),
same
> - gettext_noop("Automatic log file rotation will occur after N minutes."),
> + gettext_noop("Sets the maximum amount of time to wait before "
> + "forcing log file rotation."),
Should it say "maximum" ? Does that mean anything ?
--
Justin
On Wed, Dec 01, 2021 at 01:59:05AM -0600, Justin Pryzby wrote:
> On Tue, Nov 30, 2021 at 03:36:45PM +0900, Michael Paquier wrote:
>> - gettext_noop("Waits N seconds on connection startup before authentication."),
>> + gettext_noop("Sets the amount of seconds to wait on connection "
>> + "startup before authentication."),
>
> same
Thanks. This makes things more consistent.
>> {
>> {"checkpoint_warning", PGC_SIGHUP, WAL_CHECKPOINTS,
>> - gettext_noop("Enables warnings if checkpoint segments are filled more "
>> - "frequently than this."),
>> + gettext_noop("Sets the maximum time before warning if checkpoints "
>> + "triggered by WAL volume happen too frequently."),
>> gettext_noop("Write a message to the server log if checkpoints "
>> - "caused by the filling of checkpoint segment files happens more "
>> + "caused by the filling of WAL segment files happens more "
>
> It should say "happen" , since it's referring to "checkpoints".
> That was a pre-existing issue.
Indeed.
>> {"log_parameter_max_length", PGC_SUSET, LOGGING_WHAT,
>> - gettext_noop("When logging statements, limit logged parameter values to first N bytes."),
>> + gettext_noop("Sets the maximum amount of data logged for bind "
>> + "parameter values when logging statements."),
>
> I think this one should actually say "in bytes" or at least say "maximum
> length". It seems unlikely that someone is going to specify this in other
> units, and it's confusing to everyone else to refer to "amount of data" instead
> of "length in bytes".
Okay. Do you like the updated version attached?
>> - gettext_noop("Automatic log file rotation will occur after N minutes."),
>> + gettext_noop("Sets the maximum amount of time to wait before "
>> + "forcing log file rotation."),
>
> Should it say "maximum" ? Does that mean anything ?
To be consistent with the rest of your suggestions, we could use here:
"Sets the amount of time to wait before forcing log file rotation"
Thanks,
--
Michael
Attachment
> @@ -2142,7 +2142,8 @@ static struct config_int ConfigureNamesInt[] =
> {"post_auth_delay", PGC_BACKEND, DEVELOPER_OPTIONS,
> - gettext_noop("Waits N seconds on connection startup after authentication."),
> + gettext_noop("Sets the amount of time to wait on connection "
> + "startup after authentication."),
> @@ -2762,7 +2763,8 @@ static struct config_int ConfigureNamesInt[] =
> {"pre_auth_delay", PGC_SIGHUP, DEVELOPER_OPTIONS,
> - gettext_noop("Waits N seconds on connection startup before authentication."),
> + gettext_noop("Sets the amount of time to wait on connection "
> + "startup before authentication."),
> gettext_noop("This allows attaching a debugger to the process."),
I wonder if these should say "Sets the amount of time to wait [before]
authentication during connection startup"
> {"checkpoint_warning", PGC_SIGHUP, WAL_CHECKPOINTS,
> - gettext_noop("Enables warnings if checkpoint segments are filled more "
> - "frequently than this."),
> + gettext_noop("Sets the maximum time before warning if checkpoints "
> + "triggered by WAL volume happen too frequently."),
> gettext_noop("Write a message to the server log if checkpoints "
> - "caused by the filling of checkpoint segment files happens more "
> + "caused by the filling of WAL segment files happen more "
> "frequently than this number of seconds. Zero turns off the warning."),
Should this still say "seconds" ?
Or change it to "this amount of time"?
I'm not sure.
> {"log_rotation_age", PGC_SIGHUP, LOGGING_WHERE,
> - gettext_noop("Automatic log file rotation will occur after N minutes."),
> + gettext_noop("Sets the amount of time to wait before forcing "
> + "log file rotation."),
> NULL,
> GUC_UNIT_MIN
> },
> @@ -3154,7 +3159,8 @@ static struct config_int ConfigureNamesInt[] =
>
> {
> {"log_rotation_size", PGC_SIGHUP, LOGGING_WHERE,
> - gettext_noop("Automatic log file rotation will occur after N kilobytes."),
> + gettext_noop("Sets the maximum size of log file to reach before "
> + "forcing log file rotation."),
Actually, I think that for log_rotation_size, it should not say "forcing".
"Sets the maximum size a log file can reach before being rotated"
BTW the EXPLAIN flag for enable_incremental_sort could be backpatched to v13.
--
Justin
On Wed, Dec 01, 2021 at 09:34:39PM -0600, Justin Pryzby wrote:
>> @@ -2762,7 +2763,8 @@ static struct config_int ConfigureNamesInt[] =
>> {"pre_auth_delay", PGC_SIGHUP, DEVELOPER_OPTIONS,
>> - gettext_noop("Waits N seconds on connection startup before authentication."),
>> + gettext_noop("Sets the amount of time to wait on connection "
>> + "startup before authentication."),
>> gettext_noop("This allows attaching a debugger to the process."),
>
> I wonder if these should say "Sets the amount of time to wait [before]
> authentication during connection startup"
Hmm. I don't see much a difference between both of wordings in this
context.
>> gettext_noop("Write a message to the server log if checkpoints "
>> - "caused by the filling of checkpoint segment files happens more "
>> + "caused by the filling of WAL segment files happen more "
>> "frequently than this number of seconds. Zero turns off the warning."),
>
> Should this still say "seconds" ?
> Or change it to "this amount of time"?
> I'm not sure.
Either way would be fine by me, though I'd agree to be consistent and
use "this amount of time" here.
>> {"log_rotation_size", PGC_SIGHUP, LOGGING_WHERE,
>> - gettext_noop("Automatic log file rotation will occur after N kilobytes."),
>> + gettext_noop("Sets the maximum size of log file to reach before "
>> + "forcing log file rotation."),
>
> Actually, I think that for log_rotation_size, it should not say "forcing".
>
> "Sets the maximum size a log file can reach before being rotated"
Okay. Fine by me.
> BTW the EXPLAIN flag for enable_incremental_sort could be backpatched to v13.
This could cause small diffs in EXPLAIN outputs, which could be
surprising. This is not worth taking any risks.
--
Michael
Attachment
On Thu, Dec 02, 2021 at 02:11:38PM +0900, Michael Paquier wrote:
> On Wed, Dec 01, 2021 at 09:34:39PM -0600, Justin Pryzby wrote:
> >> @@ -2762,7 +2763,8 @@ static struct config_int ConfigureNamesInt[] =
> >> {"pre_auth_delay", PGC_SIGHUP, DEVELOPER_OPTIONS,
> >> - gettext_noop("Waits N seconds on connection startup before authentication."),
> >> + gettext_noop("Sets the amount of time to wait on connection "
> >> + "startup before authentication."),
> >> gettext_noop("This allows attaching a debugger to the process."),
> >
> > I wonder if these should say "Sets the amount of time to wait [before]
> > authentication during connection startup"
>
> Hmm. I don't see much a difference between both of wordings in this
> context.
I find it easier to read "wait before authentication ..." than "wait ... before
authentication".
> > BTW the EXPLAIN flag for enable_incremental_sort could be backpatched to v13.
>
> This could cause small diffs in EXPLAIN outputs, which could be
> surprising. This is not worth taking any risks.
Only if one specifies explain(SETTINGS).
It's fine either way ;)
--
Justin
On Wed, Dec 01, 2021 at 11:17:34PM -0600, Justin Pryzby wrote: > I find it easier to read "wait before authentication ..." than "wait ... before > authentication". I have a hard time seeing a strong difference here. At the end, I have used what you suggested, adjusted the rest based on your set of comments, and applied the patch. -- Michael
Attachment
On Fri, Dec 03, 2021 at 10:06:47AM +0900, Michael Paquier wrote: > On Wed, Dec 01, 2021 at 11:17:34PM -0600, Justin Pryzby wrote: > > I find it easier to read "wait before authentication ..." than "wait ... before > > authentication". > > I have a hard time seeing a strong difference here. At the end, I > have used what you suggested, adjusted the rest based on your set of > comments, and applied the patch. Thanks. One more item. The check_guc script currently outputs 68 false positives - even though it includes a list of 20 exceptions. This is not useful. $ (cd ./src/backend/utils/misc/; ./check_guc) |wc -l 68 With the attached: $ (cd ./src/backend/utils/misc/; ./check_guc) config_file seems to be missing from postgresql.conf.sample That has a defacto exception for the "include" directive, which seems reasonable. This requires GNU awk. I'm not sure if that's a limitation of any significance. -- Justin
Attachment
On Sun, Dec 05, 2021 at 11:38:05PM -0600, Justin Pryzby wrote: > Thanks. One more item. The check_guc script currently outputs 68 false > positives - even though it includes a list of 20 exceptions. This is not > useful. Indeed. Hmm. This script does a couple of things: 1) Check the format of the options defined in the various lists of guc.c, which is something people format well, and pgindent also does a part of this job. 2) Check that options in the hardcoded list of GUCs in INTENTIONALLY_NOT_INCLUDED are not included in postgresql.conf.sample 3) Check that nothing considered as a parameter in postgresql.conf.sample is listed in guc.c. Your patch removes 1) and 2), but keeps 3) to check for dead parameter references in postgresql.conf.sample. Is check_guc actually run on a periodic basis by somebody? Based on the amount of false positives that has accumulated over the years, and what `git grep` can already do for 3), it seems to me that we have more arguments in favor of just removing it entirely. -- Michael
Attachment
On Mon, Dec 06, 2021 at 03:58:39PM +0900, Michael Paquier wrote: > On Sun, Dec 05, 2021 at 11:38:05PM -0600, Justin Pryzby wrote: > > Thanks. One more item. The check_guc script currently outputs 68 false > > positives - even though it includes a list of 20 exceptions. This is not > > useful. > > Indeed. Hmm. This script does a couple of things: > 1) Check the format of the options defined in the various lists of > guc.c, which is something people format well, and pgindent also does > a part of this job. > 2) Check that options in the hardcoded list of GUCs in > INTENTIONALLY_NOT_INCLUDED are not included in > postgresql.conf.sample > 3) Check that nothing considered as a parameter in > postgresql.conf.sample is listed in guc.c. > > Your patch removes 1) and 2), but keeps 3) to check for dead > parameter references in postgresql.conf.sample. The script checks that guc.c and sample config are consistent. I think your undertanding of INTENTIONALLY_NOT_INCLUDED is not right. That's a list of stuff it "avoids reporting" as an suspected error, not an additional list of stuff to checks. INTENTIONALLY_NOT_INCLUDED is a list of stuff like NOT_IN_SAMPLE, which is better done by parsing /NOT_IN_SAMPLE/. > Is check_guc actually run on a periodic basis by somebody? Based on > the amount of false positives that has accumulated over the years, and > what `git grep` can already do for 3), it seems to me that we have > more arguments in favor of just removing it entirely. I saw that Tom updated it within the last 12 months, which I took to mean that it was still being maintained. But I'm okay with removing it. -- Justin
On Mon, Dec 06, 2021 at 07:36:55AM -0600, Justin Pryzby wrote: > The script checks that guc.c and sample config are consistent. > > I think your undertanding of INTENTIONALLY_NOT_INCLUDED is not right. > That's a list of stuff it "avoids reporting" as an suspected error, not an > additional list of stuff to checks. INTENTIONALLY_NOT_INCLUDED is a list of > stuff like NOT_IN_SAMPLE, which is better done by parsing /NOT_IN_SAMPLE/. Indeed. I got that wrong, thanks for clarifying. > I saw that Tom updated it within the last 12 months, which I took to mean that > it was still being maintained. But I'm okay with removing it. Yes, I saw that as of bf8a662. With 42 incorrect reports, I still see more evidence with removing it. Before doing anything, let's wait for and gather some opinions. I am adding Bruce (as the original author) and Tom in CC as they are the ones who have updated this script the most in the last ~15 years. -- Michael
Attachment
On 08.12.21 07:27, Michael Paquier wrote: >> I saw that Tom updated it within the last 12 months, which I took to mean that >> it was still being maintained. But I'm okay with removing it. > Yes, I saw that as of bf8a662. With 42 incorrect reports, I still see > more evidence with removing it. Before doing anything, let's wait for > and gather some opinions. I am adding Bruce (as the original author) > and Tom in CC as they are the ones who have updated this script the > most in the last ~15 years. I wasn't really aware of this script either. But I think it's a good idea to have it. But only if it's run automatically as part of a test suite run.
At Tue, 28 Dec 2021 20:32:40 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote in
> On Thu, Dec 09, 2021 at 09:53:23AM -0600, Justin Pryzby wrote:
> > On Thu, Dec 09, 2021 at 05:17:54PM +0900, Michael Paquier wrote:
> > One option is to expose the GUC flags in pg_settings, so this can all be done
> > in SQL regression tests.
> >
> > Maybe the flags should be text strings, so it's a nicer user-facing interface.
> > But then the field would be pretty wide, even though we're only adding it for
> > regression tests. The only other alternative I can think of is to make a
> > sql-callable function like pg_get_guc_flags(text guc).
>
> Rebased on cab5b9ab2c066ba904f13de2681872dcda31e207.
>
> And added 0003, which changes to instead exposes the flags as a function, to
> avoid changing pg_settings and exposing internally-defined integer flags in
> that somewhat prominent view.
Just an idea but couldn't we use flags in a series of one-letter flag
representations? It is more user-friendly than integers but shorter
than full-text representation.
+SELECT name, flags FROM pg_settings;
name | flags
------------------------+--------
application_name | ARsec
transaction_deferrable | Arsec
transaction_isolation | Arsec
transaction_read_only | Arsec
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 4 Jan 2022 21:06:48 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote in > On Wed, Jan 05, 2022 at 11:47:57AM +0900, Kyotaro Horiguchi wrote: > > At Tue, 28 Dec 2021 20:32:40 -0600, Justin Pryzby <pryzby@telsasoft.com> wrote in > In that case, *all* the flags should be exposed. There's currently 20, which > means it may not work well after all - it's already too long, and could get > longer, and/or overflow the alphabet... Yeah, if we show all 20 properties, the string is too long as well as all properties cannot have a sensible abbreviation character.. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On 25.01.22 02:07, Justin Pryzby wrote: > +CREATE TABLE pg_settings_flags AS SELECT name, category, > + 'NO_SHOW_ALL' =ANY(flags) AS no_show_all, > + 'NO_RESET_ALL' =ANY(flags) AS no_reset_all, > + 'NOT_IN_SAMPLE' =ANY(flags) AS not_in_sample, > + 'EXPLAIN' =ANY(flags) AS guc_explain, > + 'COMPUTED' =ANY(flags) AS guc_computed > + FROM pg_show_all_settings(); Does this stuff have any value for users? I'm worried we are exposing a bunch of stuff that is really just for internal purposes. Like, what value does showing "not_in_sample" have? On the other hand, "guc_explain" might be genuinely useful, since that is part of a user-facing feature. (I don't like the "guc_*" naming though.) Your patch doesn't contain a documentation change, so I don't know how and to what extend this is supposed to be presented to users.