Re: How about a psql backslash command to show GUCs?

2022-06-08 Thread Jonathan S. Katz

On 6/8/22 1:26 PM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

Interesting. OK, I'd say let's keep the behavior that's in the patch.


Pushed then.


Excellent -- thank you!

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-06-08 Thread Tom Lane
"Jonathan S. Katz"  writes:
> Interesting. OK, I'd say let's keep the behavior that's in the patch.

Pushed then.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-06-08 Thread Jonathan S. Katz

On 6/7/22 10:57 PM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

I don't know how frequently issues around "max_stack_depth" being too
small are reported -- I'd be curious to know that -- but I don't have
any strong arguments against allowing the behavior you describe based on
our current docs.


I can't recall any recent gripes on our own lists, but the issue was
top-of-mind for me after discovering that NetBSD defaults "ulimit -s"
to 2MB on at least some platforms.  That would leave us setting
max_stack_depth to something less than that, probably about 1.5MB.


Interesting. OK, I'd say let's keep the behavior that's in the patch.

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Tom Lane
"Jonathan S. Katz"  writes:
> I don't know how frequently issues around "max_stack_depth" being too 
> small are reported -- I'd be curious to know that -- but I don't have 
> any strong arguments against allowing the behavior you describe based on 
> our current docs.

I can't recall any recent gripes on our own lists, but the issue was
top-of-mind for me after discovering that NetBSD defaults "ulimit -s"
to 2MB on at least some platforms.  That would leave us setting
max_stack_depth to something less than that, probably about 1.5MB.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Jonathan S. Katz

On 6/7/22 7:58 PM, Tom Lane wrote:

I wrote:

The attached draft patch makes the following changes:


Here's a v2 that polishes the loose ends:


Thanks! I reviewed and did some basic testing locally. I did not see any 
of the generated defaults.



(I didn't do anything about in_hot_standby, which is set through
a hack rather than via set_config_option; not sure whether we want
to do anything there, or what it should be if we do.)


The comment diff showed that it went from "hack" to "hack" :)


I concluded that directly assigning to in_hot_standby was a fairly
horrid idea and we should just change it with SetConfigOption.
With this coding, as long as in_hot_standby is TRUE it will show
as having a non-default setting in \dconfig.  I had to remove the
assertion I'd added about PGC_INTERNAL variables only receiving
"default" values, but this just shows that was too inflexible anyway.


I tested this and the server correctly rendered "in_hot_standby" in 
\dconfig. I also tested setting "hot_standby to "on" while the server 
was not in recovery, and \dconfig correctly did not render "in_hot_standby".



* The rlimit-derived value of max_stack_depth is likewise relabeled
as PGC_S_DYNAMIC_DEFAULT, resolving the complaint Jonathan had upthread.
But now that we have a way to hide this, I'm having second thoughts
about whether we should.  If you are on a platform that's forcing an
unreasonably small stack size, it'd be good if \dconfig told you so.
Could it be sane to label that value as PGC_S_DYNAMIC_DEFAULT only when
it's the limit value (2MB) and PGC_S_ENV_VAR when it's smaller?


I concluded that was just fine and did it.


Reading the docs, I think this is OK to do. We already say that "2MB" is 
a very conservative setting. And even if the value can be computed to be 
larger, we don't allow the server to set it higher than "2MB".


I don't know how frequently issues around "max_stack_depth" being too 
small are reported -- I'd be curious to know that -- but I don't have 
any strong arguments against allowing the behavior you describe based on 
our current docs.


Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Jonathan S. Katz

On 6/7/22 1:02 PM, Tom Lane wrote:


In any case, I expect that we'd apply this patch only to HEAD, which
means that when using psql's \dconfig against a pre-v15 server,
you'd still see these settings that we're trying to hide.
That doesn't bother me too much, but maybe some would find it
confusing.


Well, "\dconfig" is a v15 feature, and though it's in the client, the 
best compatibility for it will be with v15. I think it's OK to have the 
behavior different in v15 vs. older versions.


Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Tom Lane
I wrote:
> The attached draft patch makes the following changes:

Here's a v2 that polishes the loose ends:

> (I didn't do anything about in_hot_standby, which is set through
> a hack rather than via set_config_option; not sure whether we want
> to do anything there, or what it should be if we do.)

I concluded that directly assigning to in_hot_standby was a fairly
horrid idea and we should just change it with SetConfigOption.
With this coding, as long as in_hot_standby is TRUE it will show
as having a non-default setting in \dconfig.  I had to remove the
assertion I'd added about PGC_INTERNAL variables only receiving
"default" values, but this just shows that was too inflexible anyway.

> * The rlimit-derived value of max_stack_depth is likewise relabeled
> as PGC_S_DYNAMIC_DEFAULT, resolving the complaint Jonathan had upthread.
> But now that we have a way to hide this, I'm having second thoughts
> about whether we should.  If you are on a platform that's forcing an
> unreasonably small stack size, it'd be good if \dconfig told you so.
> Could it be sane to label that value as PGC_S_DYNAMIC_DEFAULT only when
> it's the limit value (2MB) and PGC_S_ENV_VAR when it's smaller?

I concluded that was just fine and did it.

regards, tom lane

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 71136b11a2..8764084e21 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4167,7 +4167,7 @@ ReadControlFile(void)
 
 	snprintf(wal_segsz_str, sizeof(wal_segsz_str), "%d", wal_segment_size);
 	SetConfigOption("wal_segment_size", wal_segsz_str, PGC_INTERNAL,
-	PGC_S_OVERRIDE);
+	PGC_S_DYNAMIC_DEFAULT);
 
 	/* check and update variables dependent on wal_segment_size */
 	if (ConvertToXSegs(min_wal_size_mb, wal_segment_size) < 2)
@@ -4186,7 +4186,7 @@ ReadControlFile(void)
 
 	/* Make the initdb settings visible as GUC variables, too */
 	SetConfigOption("data_checksums", DataChecksumsEnabled() ? "yes" : "no",
-	PGC_INTERNAL, PGC_S_OVERRIDE);
+	PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
 }
 
 /*
@@ -4343,13 +4343,22 @@ XLOGShmemSize(void)
 	 * This isn't an amazingly clean place to do this, but we must wait till
 	 * NBuffers has received its final value, and must do it before using the
 	 * value of XLOGbuffers to do anything important.
+	 *
+	 * We prefer to report this value's source as PGC_S_DYNAMIC_DEFAULT.
+	 * However, if the DBA explicitly set wal_buffers = -1 in the config file,
+	 * then PGC_S_DYNAMIC_DEFAULT will fail to override that and we must force
+	 * the matter with PGC_S_OVERRIDE.
 	 */
 	if (XLOGbuffers == -1)
 	{
 		char		buf[32];
 
 		snprintf(buf, sizeof(buf), "%d", XLOGChooseNumBuffers());
-		SetConfigOption("wal_buffers", buf, PGC_POSTMASTER, PGC_S_OVERRIDE);
+		SetConfigOption("wal_buffers", buf, PGC_POSTMASTER,
+		PGC_S_DYNAMIC_DEFAULT);
+		if (XLOGbuffers == -1)	/* failed to apply it? */
+			SetConfigOption("wal_buffers", buf, PGC_POSTMASTER,
+			PGC_S_OVERRIDE);
 	}
 	Assert(XLOGbuffers > 0);
 
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 9fa8fdd4cf..9a610d41ad 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -262,7 +262,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
  errmsg("-X requires a power of two value between 1 MB and 1 GB")));
 	SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL,
-	PGC_S_OVERRIDE);
+	PGC_S_DYNAMIC_DEFAULT);
 }
 break;
 			case 'c':
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3b73e26956..dde4bc25b1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -921,7 +921,7 @@ PostmasterMain(int argc, char *argv[])
 		 * useful to show, even if one would only expect at least PANIC.  LOG
 		 * entries are hidden.
 		 */
-		SetConfigOption("log_min_messages", "FATAL", PGC_INTERNAL,
+		SetConfigOption("log_min_messages", "FATAL", PGC_SUSET,
 		PGC_S_OVERRIDE);
 	}
 
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 26372d95b3..1a6f527051 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -334,7 +334,8 @@ InitializeShmemGUCs(void)
 	size_b = CalculateShmemSize(NULL);
 	size_mb = add_size(size_b, (1024 * 1024) - 1) / (1024 * 1024);
 	sprintf(buf, "%zu", size_mb);
-	SetConfigOption("shared_memory_size", buf, PGC_INTERNAL, PGC_S_OVERRIDE);
+	SetConfigOption("shared_memory_size", buf,
+	PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
 
 	/*
 	 * Calculate the number of huge pages required.
@@ -346,6 +347,7 @@ InitializeShmemGUCs(void)
 
 		hp_required = add_size(size_b / hp_size, 1);
 		sprintf(buf, "%zu", hp_required);
-		SetConfigOption("shared_memory_size_in_huge_pages", buf, PGC_INTERNAL, PGC_S_OVERRIDE);
+		SetConfigOption(

Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Tom Lane
Christoph Berg  writes:
> in_hot_standby sounds very useful to have in that list.

I thought about this some more and concluded that we're blaming the
messenger.  There's nothing wrong with \dconfig's filtering rule;
rather, it's the fault of the backend for mislabeling a lot of
run-time-computed values as source=PGC_S_OVERRIDE when they really
are dynamic defaults.  Now in fairness, I think a lot of that code
predates the invention of PGC_S_DYNAMIC_DEFAULT, so there was not
a better way when it was written ... but there is now.

The attached draft patch makes the following changes:

* All run-time calculations of PGC_INTERNAL variables are relabeled
as PGC_S_DYNAMIC_DEFAULT.  There is not any higher source value
that we'd allow to set such a variable, so this is sufficient.
(I didn't do anything about in_hot_standby, which is set through
a hack rather than via set_config_option; not sure whether we want
to do anything there, or what it should be if we do.)  The net
effect of this compared to upthread examples is to hide
shared_memory_size and shared_memory_size_in_huge_pages from \dconfig.

* The auto-tune value of wal_buffers is relabeled as PGC_S_DYNAMIC_DEFAULT
if possible (but due to pre-existing hacks, we might have to force it).
This will hide that one too as long as you didn't manually set it.

* The rlimit-derived value of max_stack_depth is likewise relabeled
as PGC_S_DYNAMIC_DEFAULT, resolving the complaint Jonathan had upthread.
But now that we have a way to hide this, I'm having second thoughts
about whether we should.  If you are on a platform that's forcing an
unreasonably small stack size, it'd be good if \dconfig told you so.
Could it be sane to label that value as PGC_S_DYNAMIC_DEFAULT only when
it's the limit value (2MB) and PGC_S_ENV_VAR when it's smaller?

In any case, I expect that we'd apply this patch only to HEAD, which
means that when using psql's \dconfig against a pre-v15 server,
you'd still see these settings that we're trying to hide.
That doesn't bother me too much, but maybe some would find it
confusing.

Thoughts?

regards, tom lane

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 71136b11a2..2461317ee4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4167,7 +4167,7 @@ ReadControlFile(void)
 
 	snprintf(wal_segsz_str, sizeof(wal_segsz_str), "%d", wal_segment_size);
 	SetConfigOption("wal_segment_size", wal_segsz_str, PGC_INTERNAL,
-	PGC_S_OVERRIDE);
+	PGC_S_DYNAMIC_DEFAULT);
 
 	/* check and update variables dependent on wal_segment_size */
 	if (ConvertToXSegs(min_wal_size_mb, wal_segment_size) < 2)
@@ -4186,7 +4186,7 @@ ReadControlFile(void)
 
 	/* Make the initdb settings visible as GUC variables, too */
 	SetConfigOption("data_checksums", DataChecksumsEnabled() ? "yes" : "no",
-	PGC_INTERNAL, PGC_S_OVERRIDE);
+	PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
 }
 
 /*
@@ -4343,13 +4343,22 @@ XLOGShmemSize(void)
 	 * This isn't an amazingly clean place to do this, but we must wait till
 	 * NBuffers has received its final value, and must do it before using the
 	 * value of XLOGbuffers to do anything important.
+	 *
+	 * We prefer to report this value as PGC_S_DYNAMIC_DEFAULT.  However, if
+	 * the DBA explicitly set wal_buffers = -1 in the config file, then
+	 * PGC_S_DYNAMIC_DEFAULT will fail to override that and we must force the
+	 * matter with PGC_S_OVERRIDE.
 	 */
 	if (XLOGbuffers == -1)
 	{
 		char		buf[32];
 
 		snprintf(buf, sizeof(buf), "%d", XLOGChooseNumBuffers());
-		SetConfigOption("wal_buffers", buf, PGC_POSTMASTER, PGC_S_OVERRIDE);
+		SetConfigOption("wal_buffers", buf, PGC_POSTMASTER,
+		PGC_S_DYNAMIC_DEFAULT);
+		if (XLOGbuffers == -1)	/* failed to apply it? */
+			SetConfigOption("wal_buffers", buf, PGC_POSTMASTER,
+			PGC_S_OVERRIDE);
 	}
 	Assert(XLOGbuffers > 0);
 
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 9fa8fdd4cf..9a610d41ad 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -262,7 +262,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
  errmsg("-X requires a power of two value between 1 MB and 1 GB")));
 	SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL,
-	PGC_S_OVERRIDE);
+	PGC_S_DYNAMIC_DEFAULT);
 }
 break;
 			case 'c':
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3b73e26956..dde4bc25b1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -921,7 +921,7 @@ PostmasterMain(int argc, char *argv[])
 		 * useful to show, even if one would only expect at least PANIC.  LOG
 		 * entries are hidden.
 		 */
-		SetConfigOption("log_min_messages", "FATAL", PGC_INTERNAL,
+		SetConfigOption("log_min_messages", "FATAL", PGC_SUSET,
 		PGC_S_OVERRIDE);
 	

Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Christoph Berg
Re: Jonathan S. Katz
> On 6/7/22 10:26 AM, Robert Haas wrote:
> I think some of these could be interesting if they deviate from the default
> (e.g. "in_hot_standby") as it will give the user context on the current
> state of the system.
> 
> However, something like that is still fairly easy to determine (e.g.
> `pg_catalog.pg_is_in_recovery()`). And looking through the settings marked
> "internal" showing the non-defaults may not provide much additional context
> to a user.

in_hot_standby sounds very useful to have in that list.

Christoph




Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Jonathan S. Katz

On 6/7/22 10:26 AM, Robert Haas wrote:

On Mon, Jun 6, 2022 at 5:02 PM Tom Lane  wrote:

I think a reasonable case can be made for excluding "internal" GUCs
on the grounds that (a) they cannot be set, and therefore (b) whatever
value they have might as well be considered the default.


I agree.


I think some of these could be interesting if they deviate from the 
default (e.g. "in_hot_standby") as it will give the user context on the 
current state of the system.


However, something like that is still fairly easy to determine (e.g. 
`pg_catalog.pg_is_in_recovery()`). And looking through the settings 
marked "internal" showing the non-defaults may not provide much 
additional context to a user.


+1 for excluding them.

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-06-07 Thread Robert Haas
On Mon, Jun 6, 2022 at 5:02 PM Tom Lane  wrote:
> I think a reasonable case can be made for excluding "internal" GUCs
> on the grounds that (a) they cannot be set, and therefore (b) whatever
> value they have might as well be considered the default.

I agree.

-- 
Robert Haas
EDB: http://www.enterprisedb.com




Re: How about a psql backslash command to show GUCs?

2022-06-06 Thread Tom Lane
Robert Haas  writes:
> I think part of the problem here, though, is that one can imagine a
> variety of charters that might be useful. A user could want to see the
> parameters that have values in their session that differ from the
> system defaults, or parameters that have values which differ from the
> compiled-in defaults, or parameters that can be changed without a
> restart, or all the pre-computed parameters, or all the parameters
> that contain "vacuum" in the name, or all the query-planner-related
> parameters, or all the parameters on which any privileges have been
> granted. And it's just a judgement call which of those things we ought
> to try to accommodate in the psql syntax and which ones ought to be
> done by writing an ad-hoc query against pg_settings.

Sure.  Nonetheless, having decided to introduce this command, we have
to make that judgement call.

psql-ref.sgml currently explains that

If pattern is specified,
only parameters whose names match the pattern are listed.  Without
a pattern, only
parameters that are set to non-default values are listed.
(Use \dconfig * to see all parameters.)

so we have the "all of them" and "ones whose names match a pattern"
cases covered.  And the definition of the default behavior as
"only ones that are set to non-default values" seems reasonable enough,
but the question is what counts as a "non-default value", or for that
matter what counts as "setting".

I think a reasonable case can be made for excluding "internal" GUCs
on the grounds that (a) they cannot be set, and therefore (b) whatever
value they have might as well be considered the default.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-06-06 Thread Mark Dilger



> On Jun 6, 2022, at 9:02 AM, Tom Lane  wrote:
> 
> Thoughts?

I think it depends on your mental model of what \dconfig is showing you.  Is it 
showing you the list of configs that you can SET, or just the list of all 
configs?





Re: How about a psql backslash command to show GUCs?

2022-06-06 Thread Robert Haas
On Mon, Jun 6, 2022 at 12:02 PM Tom Lane  wrote:
> Thoughts?

This all seems pretty subjective. As someone who sometimes supports
customers, I usually kind of want the customer to give me all the
settings, just in case something that didn't seem important to them
(or to whoever coded up the \dconfig command) turns out to be
relevant. It's easier to ask once and then look for the information
you need than to go back and forth asking for more data, and I don't
want to have to try to infer things based on what version they are
running or how a certain set of binaries was built.

But if I already know that the configuration on a given system is
basically sane, I probably only care about the parameters with
non-default values, and a computed parameter can't be set, so I guess
it has a default value by definition. So if the charter for this
command is to show only non-default GUCs, then it seems reasonable to
leave these out.

I think part of the problem here, though, is that one can imagine a
variety of charters that might be useful. A user could want to see the
parameters that have values in their session that differ from the
system defaults, or parameters that have values which differ from the
compiled-in defaults, or parameters that can be changed without a
restart, or all the pre-computed parameters, or all the parameters
that contain "vacuum" in the name, or all the query-planner-related
parameters, or all the parameters on which any privileges have been
granted. And it's just a judgement call which of those things we ought
to try to accommodate in the psql syntax and which ones ought to be
done by writing an ad-hoc query against pg_settings.

-- 
Robert Haas
EDB: http://www.enterprisedb.com




Re: How about a psql backslash command to show GUCs?

2022-06-06 Thread Tom Lane
Justin Pryzby  writes:
> I noticed this is showing "pre-computed" gucs, like:
>  shared_memory_size| 149MB
>  shared_memory_size_in_huge_pages  | 75
> I'm not opposed to that, but I wonder if that's what's intended / best.

I had suggested upthread that we might want to hide items with
source = 'override', but that idea didn't seem to be getting traction.
A different idea is to hide items with context = 'internal'.
Looking at the items selected by the current rule in a default
installation:

postgres=# SELECT s.name, source, context FROM pg_catalog.pg_settings s
WHERE s.source <> 'default' AND
  s.setting IS DISTINCT FROM s.boot_val
ORDER BY 1;
   name   |source|  context   
--+--+
 TimeZone | configuration file   | user
 application_name | client   | user
 client_encoding  | client   | user
 config_file  | override | postmaster
 data_directory   | override | postmaster
 default_text_search_config   | configuration file   | user
 hba_file | override | postmaster
 ident_file   | override | postmaster
 lc_messages  | configuration file   | superuser
 log_timezone | configuration file   | sighup
 max_stack_depth  | environment variable | superuser
 shared_memory_size   | override | internal
 shared_memory_size_in_huge_pages | override | internal
 wal_buffers  | override | postmaster
(14 rows)

So hiding internal-context items would hit exactly the two you mention,
but hiding override-source items would hit several more.

(I'm kind of wondering why wal_buffers is showing as "override";
that seems like a quirk.)

Thoughts?

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-06-06 Thread Justin Pryzby
On Tue, Apr 12, 2022 at 11:19:40AM -0400, Tom Lane wrote:
> "Jonathan S. Katz"  writes:
> > On 4/11/22 4:11 PM, Tom Lane wrote:
> >> This idea does somewhat address my unhappiness upthread about printing
> >> values with source = 'internal', but I see that it gets confused by
> >> some GUCs with custom show hooks, like unix_socket_permissions.
> >> Maybe it needs to be "source != 'default' AND setting != boot_val"?
> 
> > Running through a few GUCs, that seems reasonable. Happy to test the 
> > patch out prior to commit to see if it renders better.
> 
> It'd just look like this, I think.  I see from looking at guc.c that
> boot_val can be NULL, so we'd better use IS DISTINCT FROM.

I noticed this is showing "pre-computed" gucs, like:

 shared_memory_size| 149MB
 shared_memory_size_in_huge_pages  | 75

I'm not opposed to that, but I wonder if that's what's intended / best.

-- 
Justin




Re: How about a psql backslash command to show GUCs?

2022-04-13 Thread Tom Lane
"Jonathan S. Katz"  writes:
> On 4/12/22 1:00 PM, Tom Lane wrote:
>> Yeah, most of what shows up in a minimally-configured installation is
>> postmaster-computed settings like config_file, rather than things
>> that were actually set by the DBA.  Personally I'd rather hide the
>> ones that have source = 'override', but that didn't seem to be the
>> consensus.

> The list seems more reasonable now, though now that I'm fully in the 
> "less is more" camp based on the "non-defaults" description, I think 
> anything we can do to further prune is good.

Hearing no further comments, I pushed this version.  There didn't seem
to be a need to adjust the docs.

> We may be at a point where it's "good enough" and let more people chime 
> in during beta.

Right, lots of time yet for bikeshedding in beta.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-12 Thread Jonathan S. Katz

On 4/12/22 1:00 PM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

On 4/12/22 11:19 AM, Tom Lane wrote:

It'd just look like this, I think.  I see from looking at guc.c that
boot_val can be NULL, so we'd better use IS DISTINCT FROM.



I tested it and I like this a lot better, at least it's much more
consolidated. They all seem to be generated (directories, timezones,
collations/encodings).


Yeah, most of what shows up in a minimally-configured installation is
postmaster-computed settings like config_file, rather than things
that were actually set by the DBA.  Personally I'd rather hide the
ones that have source = 'override', but that didn't seem to be the
consensus.


The list seems more reasonable now, though now that I'm fully in the 
"less is more" camp based on the "non-defaults" description, I think 
anything we can do to further prune is good.



The one exception to this seems to be "max_stack_depth", which is
rendering on my "\dconfig" though I didn't change it, an it's showing
it's default value of 2MB. "boot_val" says 100, "reset_val" says 2048,
and it's commented out in my postgresql.conf. Do we want to align that?


I don't think there's any principled thing we could do about that in
psql.  The boot_val is a conservatively small 100kB, but we crank
that up automatically based on getrlimit(RLIMIT_STACK), so on any
reasonable platform it's going to show as not being default.


Got it.

We may be at a point where it's "good enough" and let more people chime 
in during beta.


Thanks,

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-12 Thread Tom Lane
"Jonathan S. Katz"  writes:
> On 4/12/22 11:19 AM, Tom Lane wrote:
>> It'd just look like this, I think.  I see from looking at guc.c that
>> boot_val can be NULL, so we'd better use IS DISTINCT FROM.

> I tested it and I like this a lot better, at least it's much more 
> consolidated. They all seem to be generated (directories, timezones, 
> collations/encodings).

Yeah, most of what shows up in a minimally-configured installation is
postmaster-computed settings like config_file, rather than things
that were actually set by the DBA.  Personally I'd rather hide the
ones that have source = 'override', but that didn't seem to be the
consensus.

> The one exception to this seems to be "max_stack_depth", which is 
> rendering on my "\dconfig" though I didn't change it, an it's showing 
> it's default value of 2MB. "boot_val" says 100, "reset_val" says 2048, 
> and it's commented out in my postgresql.conf. Do we want to align that?

I don't think there's any principled thing we could do about that in
psql.  The boot_val is a conservatively small 100kB, but we crank
that up automatically based on getrlimit(RLIMIT_STACK), so on any
reasonable platform it's going to show as not being default.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-12 Thread Jonathan S. Katz

On 4/12/22 11:19 AM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

On 4/11/22 4:11 PM, Tom Lane wrote:

This idea does somewhat address my unhappiness upthread about printing
values with source = 'internal', but I see that it gets confused by
some GUCs with custom show hooks, like unix_socket_permissions.
Maybe it needs to be "source != 'default' AND setting != boot_val"?



Running through a few GUCs, that seems reasonable. Happy to test the
patch out prior to commit to see if it renders better.


It'd just look like this, I think.  I see from looking at guc.c that
boot_val can be NULL, so we'd better use IS DISTINCT FROM.


(IS DISTINCT FROM is pretty handy :)

I tested it and I like this a lot better, at least it's much more 
consolidated. They all seem to be generated (directories, timezones, 
collations/encodings).


The one exception to this seems to be "max_stack_depth", which is 
rendering on my "\dconfig" though I didn't change it, an it's showing 
it's default value of 2MB. "boot_val" says 100, "reset_val" says 2048, 
and it's commented out in my postgresql.conf. Do we want to align that?


That said, the patch itself LGTM.

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-12 Thread Tom Lane
"Jonathan S. Katz"  writes:
> On 4/11/22 4:11 PM, Tom Lane wrote:
>> This idea does somewhat address my unhappiness upthread about printing
>> values with source = 'internal', but I see that it gets confused by
>> some GUCs with custom show hooks, like unix_socket_permissions.
>> Maybe it needs to be "source != 'default' AND setting != boot_val"?

> Running through a few GUCs, that seems reasonable. Happy to test the 
> patch out prior to commit to see if it renders better.

It'd just look like this, I think.  I see from looking at guc.c that
boot_val can be NULL, so we'd better use IS DISTINCT FROM.

regards, tom lane

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e7377d4583..17790bd8a4 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -4410,7 +4410,8 @@ describeConfigurationParameters(const char *pattern, bool verbose,
 			  NULL, "pg_catalog.lower(s.name)", NULL,
 			  NULL);
 	else
-		appendPQExpBufferStr(&buf, "WHERE s.source <> 'default'\n");
+		appendPQExpBufferStr(&buf, "WHERE s.source <> 'default' AND\n"
+			 "  s.setting IS DISTINCT FROM s.boot_val\n");
 
 	appendPQExpBufferStr(&buf, "ORDER BY 1;");
 


Re: How about a psql backslash command to show GUCs?

2022-04-11 Thread Jonathan S. Katz

On 4/11/22 4:11 PM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

My question is if we're only going to list out the settings that are
customized, are we going to:



1. Hide a setting if it matches a default value, even if a user set it
to be the default value? OR
2. Comment out all of the settings in a generated postgresql.conf file?


As committed, it prints anything that's shown as "source != 'default'"
in pg_settings, which means anything for which the value wasn't
taken from the wired-in default.  I suppose an alternative definition
could be "setting != boot_val".  Not really sure if that's better.

This idea does somewhat address my unhappiness upthread about printing
values with source = 'internal', but I see that it gets confused by
some GUCs with custom show hooks, like unix_socket_permissions.
Maybe it needs to be "source != 'default' AND setting != boot_val"?


Running through a few GUCs, that seems reasonable. Happy to test the 
patch out prior to commit to see if it renders better.


Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-11 Thread Tom Lane
Christoph Berg  writes:
> Plus maybe making initdb not set values to their default if the auto probing 
> ends up at that values.

Seems a bit fragile: we'd have to make sure that initdb knew what the
boot_val is.  IIRC, some of those are not necessarily immutable constants,
so there'd be room for error.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-11 Thread Christoph Berg
Plus maybe making initdb not set values to their default if the auto probing 
ends up at that values.

Christoph

Re: How about a psql backslash command to show GUCs?

2022-04-11 Thread Tom Lane
"Jonathan S. Katz"  writes:
> My question is if we're only going to list out the settings that are 
> customized, are we going to:

> 1. Hide a setting if it matches a default value, even if a user set it 
> to be the default value? OR
> 2. Comment out all of the settings in a generated postgresql.conf file?

As committed, it prints anything that's shown as "source != 'default'"
in pg_settings, which means anything for which the value wasn't
taken from the wired-in default.  I suppose an alternative definition
could be "setting != boot_val".  Not really sure if that's better.

This idea does somewhat address my unhappiness upthread about printing
values with source = 'internal', but I see that it gets confused by
some GUCs with custom show hooks, like unix_socket_permissions.
Maybe it needs to be "source != 'default' AND setting != boot_val"?

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-11 Thread Jonathan S. Katz

On 4/11/22 3:12 PM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

On 4/9/22 12:27 PM, Tom Lane wrote:

Sure, but then you do "\dconfig *".  With there being several hundred
GUCs (and no doubt more coming), I'm not sure that "show me every GUC"
is a common use-case at all, let alone so common as to deserve being
the default behavior.

One thing we could perhaps do to reduce confusion is to change the
table heading when doing this, say from "List of configuration parameters"
to "List of non-default configuration parameters".



Reasonable points. I don't have any objections to this proposal.


Hearing no further comments, done like that.


Thanks!

I have a usability comment based on my testing.

I ran "\dconfig" and one of the settings that came up was 
"max_connections" which was set to 100, or the default.


I had noticed that "max_connections" was uncommented in my 
postgresql.conf file, which I believe was from the autogenerated 
provided by initdb.


Similarly, min_wal_size/max_wal_size were in the list and set to their 
default values. These were also uncommented in my postgresql.conf from 
the autogeneration.


My question is if we're only going to list out the settings that are 
customized, are we going to:


1. Hide a setting if it matches a default value, even if a user set it 
to be the default value? OR

2. Comment out all of the settings in a generated postgresql.conf file?

Thanks,

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-11 Thread Tom Lane
"Jonathan S. Katz"  writes:
> On 4/9/22 12:27 PM, Tom Lane wrote:
>> Sure, but then you do "\dconfig *".  With there being several hundred
>> GUCs (and no doubt more coming), I'm not sure that "show me every GUC"
>> is a common use-case at all, let alone so common as to deserve being
>> the default behavior.
>> 
>> One thing we could perhaps do to reduce confusion is to change the
>> table heading when doing this, say from "List of configuration parameters"
>> to "List of non-default configuration parameters".

> Reasonable points. I don't have any objections to this proposal.

Hearing no further comments, done like that.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Jonathan S. Katz

On 4/9/22 12:27 PM, Tom Lane wrote:

"Jonathan S. Katz"  writes:

-1, at least for the moment. Sometimes a user doesn't know what they're
looking for coupled with being unaware of what the default value is. If
a setting is set to a default value and that value is the problematic
setting, a user should be able to see that even in a full list.


Sure, but then you do "\dconfig *".  With there being several hundred
GUCs (and no doubt more coming), I'm not sure that "show me every GUC"
is a common use-case at all, let alone so common as to deserve being
the default behavior.

One thing we could perhaps do to reduce confusion is to change the
table heading when doing this, say from "List of configuration parameters"
to "List of non-default configuration parameters".


Reasonable points. I don't have any objections to this proposal.

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Vik Fearing

On 4/9/22 16:31, Tom Lane wrote:

Christoph Berg  writes:


I would think that if \dconfig showed the non-default settings only,
it would be much more useful; the full list would still be available
with "\dconfig *". This is in line with \dt only showing tables on the
search_path, and "\dt *.*" showing all.


Hm, I could get on board with that -- any other opinions?


+1
--
Vik Fearing




Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Robert Haas
On Sat, Apr 9, 2022 at 10:31 AM Tom Lane  wrote:
> Christoph Berg  writes:
> > The name has evolved from \dcp over various longer \d-things to the
> > more verbose \dconfig. How about we evolve it even more and just call
> > it \config?
>
> I think people felt that it should be part of the \d family.
> Also, because we have \connect and \conninfo, you'd need to
> type at least five characters before you could tab-complete,
> whereas \dconfig is unique at four (you just need \dco).

Regarding this point, I kind of think that \dconfig is a break with
established precedent, but in a good way. Previous additions have
generally tried to pick some vaguely mnemonic sequence of letters that
somehow corresponds to what's being listed, but you have to be pretty
hard-core to remember what \db and \dAc and \drds do. \dconfig is
probably easier to remember.

-- 
Robert Haas
EDB: http://www.enterprisedb.com




Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread David G. Johnston
On Sat, Apr 9, 2022 at 9:27 AM Tom Lane  wrote:

> "Jonathan S. Katz"  writes:
> > -1, at least for the moment. Sometimes a user doesn't know what they're
> > looking for coupled with being unaware of what the default value is. If
> > a setting is set to a default value and that value is the problematic
> > setting, a user should be able to see that even in a full list.
>
> Sure, but then you do "\dconfig *".  With there being several hundred
> GUCs (and no doubt more coming), I'm not sure that "show me every GUC"
> is a common use-case at all, let alone so common as to deserve being
> the default behavior.
>
>
I'm for having a default that doesn't mean "show everything".

I'm also wondering whether we can invent GUC namespaces for the different
contexts, so I can use a pattern like: context.internal.*

A similar ability for category would be nice but we'd have to invent labels
to make it practical.

\dconfig [pattern [mode]]

mode: all, overridden

So mode is overridden if pattern is absent but all if pattern is present,
with the ability to specify overridden.

pattern: [[{context.{context label}}|{category.{category
label}}.]...]{parameter name pattern}
parameter name pattern: [{two part name prefix}.]{base parameter pattern}


One thing we could perhaps do to reduce confusion is to change the
> table heading when doing this, say from "List of configuration parameters"
> to "List of non-default configuration parameters".
>
>
I'd be inclined to echo a note after the output table that says that not
all configuration parameters are displayed - possibly even providing a
count [all - overridden].  The header is likely to be ignored even if it
still ends up on screen after scrolling.

David J.


Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Tom Lane
"Jonathan S. Katz"  writes:
> -1, at least for the moment. Sometimes a user doesn't know what they're 
> looking for coupled with being unaware of what the default value is. If 
> a setting is set to a default value and that value is the problematic 
> setting, a user should be able to see that even in a full list.

Sure, but then you do "\dconfig *".  With there being several hundred
GUCs (and no doubt more coming), I'm not sure that "show me every GUC"
is a common use-case at all, let alone so common as to deserve being
the default behavior.

One thing we could perhaps do to reduce confusion is to change the
table heading when doing this, say from "List of configuration parameters"
to "List of non-default configuration parameters".

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Jonathan S. Katz

On 4/9/22 11:58 AM, Julien Rouhaud wrote:

On Sat, Apr 09, 2022 at 10:31:17AM -0400, Tom Lane wrote:

Christoph Berg  writes:


I would think that if \dconfig showed the non-default settings only,
it would be much more useful; the full list would still be available
with "\dconfig *". This is in line with \dt only showing tables on the
search_path, and "\dt *.*" showing all.


Hm, I could get on board with that -- any other opinions?


+1 for it, that's often what I'm interested in when looking at the GUCs in
general.


-1, at least for the moment. Sometimes a user doesn't know what they're 
looking for coupled with being unaware of what the default value is. If 
a setting is set to a default value and that value is the problematic 
setting, a user should be able to see that even in a full list.


(The \dt searching only tables "search_path" vs. the database has also 
bitten me too. I did ultimately learn about "\dt *.*", but this makes 
the user have to unpack more layers to do simple things).


Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Julien Rouhaud
On Sat, Apr 09, 2022 at 10:31:17AM -0400, Tom Lane wrote:
> Christoph Berg  writes:
> 
> > I would think that if \dconfig showed the non-default settings only,
> > it would be much more useful; the full list would still be available
> > with "\dconfig *". This is in line with \dt only showing tables on the
> > search_path, and "\dt *.*" showing all.
> 
> Hm, I could get on board with that -- any other opinions?

+1 for it, that's often what I'm interested in when looking at the GUCs in
general.

> (Perhaps there's an argument for omitting "override"
> settings as well?)

-0.1.  Most are usually not useful, but I can see at least data_checksums and
wal_buffers that are still interesting.




Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Tom Lane
Christoph Berg  writes:
> The name has evolved from \dcp over various longer \d-things to the
> more verbose \dconfig. How about we evolve it even more and just call
> it \config?

I think people felt that it should be part of the \d family.
Also, because we have \connect and \conninfo, you'd need to
type at least five characters before you could tab-complete,
whereas \dconfig is unique at four (you just need \dco).

> I would think that if \dconfig showed the non-default settings only,
> it would be much more useful; the full list would still be available
> with "\dconfig *". This is in line with \dt only showing tables on the
> search_path, and "\dt *.*" showing all.

Hm, I could get on board with that -- any other opinions?
(Perhaps there's an argument for omitting "override"
settings as well?)

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-09 Thread Christoph Berg
Re: Tom Lane
> Looks like the consensus has shifted to \dconfig.  I'll do it like that.

A bit late to the party, but two more ideas:


The name has evolved from \dcp over various longer \d-things to the
more verbose \dconfig. How about we evolve it even more and just call
it \config? That would be much easier to remember - in fact after I
seeing the patch the other day, I wanted to try it today and I was
confused when \config didn't work, and had to read the git log to see
how it's actually called.

It also doesn't conflict with tab completion too much, \conf
would work.


The other bit is hiding non-default values. "\dconfig" by itself is
very long and not very interesting. I have this in my .psqlrc that I
use very often on servers I'm visiting:

\set config 'SELECT name, current_setting(name), CASE source WHEN 
$$configuration file$$ THEN regexp_replace(sourcefile, $$^/.*/$$, 
)||$$:$$||sourceline ELSE source END FROM pg_settings WHERE source <> 
$$default$$;'

I would think that if \dconfig showed the non-default settings only,
it would be much more useful; the full list would still be available
with "\dconfig *". This is in line with \dt only showing tables on the
search_path, and "\dt *.*" showing all.

Christoph




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Tom Lane
Pavel Stehule  writes:
> čt 7. 4. 2022 v 19:04 odesílatel David G. Johnston <
> david.g.johns...@gmail.com> napsal:
>> \dconfig[+] gets my vote.  I was going to say "conf" just isn't common
>> jargon to say or write; but the one place it is - file extensions - is
>> relevant and common.  But still, I would go with the non-jargon form.

> dconfig is better, because google can be confused - dconf is known project
> https://en.wikipedia.org/wiki/Dconf

Looks like the consensus has shifted to \dconfig.  I'll do it like that.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Pavel Stehule
čt 7. 4. 2022 v 19:04 odesílatel David G. Johnston <
david.g.johns...@gmail.com> napsal:

> On Thu, Apr 7, 2022 at 9:58 AM Joe Conway  wrote:
>
>> On 4/7/22 12:37, Tom Lane wrote:
>> > Mark Dilger  writes:
>> >>> On Apr 7, 2022, at 9:29 AM, Tom Lane  wrote:
>> >>> I wouldn't
>> >>> fight too hard if people want to lengthen it to \dconfig for
>> consistency
>> >>> with set_config().
>> >
>> >> I'd prefer \dconfig, but if the majority on this list view that as
>> pedantically forcing them to type more, I'm not going to kick up a fuss
>> about \dconf.
>> >
>> > Maybe I'm atypical, but I'm probably going to use tab completion
>> > either way, so it's not really more keystrokes.  The consistency
>> > point is a good one that I'd not considered before.
>>
>> Yeah I had thought about \dconfig too -- +1 to that, although I am fine
>> with \dconf too.
>>
>>
> \dconfig[+] gets my vote.  I was going to say "conf" just isn't common
> jargon to say or write; but the one place it is - file extensions - is
> relevant and common.  But still, I would go with the non-jargon form.
>

dconfig is better, because google can be confused - dconf is known project
https://en.wikipedia.org/wiki/Dconf

The length is not too important when we have tab complete

Regards

Pavel


> David J.
>


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread David G. Johnston
On Thu, Apr 7, 2022 at 9:58 AM Joe Conway  wrote:

> On 4/7/22 12:37, Tom Lane wrote:
> > Mark Dilger  writes:
> >>> On Apr 7, 2022, at 9:29 AM, Tom Lane  wrote:
> >>> I wouldn't
> >>> fight too hard if people want to lengthen it to \dconfig for
> consistency
> >>> with set_config().
> >
> >> I'd prefer \dconfig, but if the majority on this list view that as
> pedantically forcing them to type more, I'm not going to kick up a fuss
> about \dconf.
> >
> > Maybe I'm atypical, but I'm probably going to use tab completion
> > either way, so it's not really more keystrokes.  The consistency
> > point is a good one that I'd not considered before.
>
> Yeah I had thought about \dconfig too -- +1 to that, although I am fine
> with \dconf too.
>
>
\dconfig[+] gets my vote.  I was going to say "conf" just isn't common
jargon to say or write; but the one place it is - file extensions - is
relevant and common.  But still, I would go with the non-jargon form.

David J.


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Joe Conway

On 4/7/22 12:37, Tom Lane wrote:

Mark Dilger  writes:

On Apr 7, 2022, at 9:29 AM, Tom Lane  wrote:
I wouldn't
fight too hard if people want to lengthen it to \dconfig for consistency
with set_config().



I'd prefer \dconfig, but if the majority on this list view that as pedantically 
forcing them to type more, I'm not going to kick up a fuss about \dconf.


Maybe I'm atypical, but I'm probably going to use tab completion
either way, so it's not really more keystrokes.  The consistency
point is a good one that I'd not considered before.


Yeah I had thought about \dconfig too -- +1 to that, although I am fine 
with \dconf too.


Joe




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Jonathan S. Katz

On 4/7/22 12:42 PM, Mark Dilger wrote:




On Apr 7, 2022, at 9:37 AM, Tom Lane  wrote:

Maybe I'm atypical, but I'm probably going to use tab completion
either way, so it's not really more keystrokes.


Same here, because after tab-completing \dcon\t\t into \dconfig, I'm likely to 
also tab-complete to get the list of parameters.  I frequently can't recall the 
exact spelling of them.


This seems like the only "\d" command that would require tab completion, 
given all the others are far less descriptive (\dt, \dv, etc.) And at 
least from my user perspective, if I ever need anything other than \dt, 
I typically have to go to \? to look it up.


I'm generally in favor of consistency, though in case skewing towards 
what we expose to the user. If "\dconfig" gives a bit more across the 
board, I'm OK with that. "\dparam" could be a bit confusing to end users 
("parameter for what?") so I'm -1 on that.


Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Mark Dilger



> On Apr 7, 2022, at 9:37 AM, Tom Lane  wrote:
> 
> Maybe I'm atypical, but I'm probably going to use tab completion
> either way, so it's not really more keystrokes.

Same here, because after tab-completing \dcon\t\t into \dconfig, I'm likely to 
also tab-complete to get the list of parameters.  I frequently can't recall the 
exact spelling of them.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company







Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Tom Lane
Mark Dilger  writes:
>> On Apr 7, 2022, at 9:29 AM, Tom Lane  wrote:
>> I wouldn't
>> fight too hard if people want to lengthen it to \dconfig for consistency
>> with set_config().

> I'd prefer \dconfig, but if the majority on this list view that as 
> pedantically forcing them to type more, I'm not going to kick up a fuss about 
> \dconf.

Maybe I'm atypical, but I'm probably going to use tab completion
either way, so it's not really more keystrokes.  The consistency
point is a good one that I'd not considered before.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Mark Dilger



> On Apr 7, 2022, at 9:29 AM, Tom Lane  wrote:
> 
> I wouldn't
> fight too hard if people want to lengthen it to \dconfig for consistency
> with set_config().

I'd prefer \dconfig, but if the majority on this list view that as pedantically 
forcing them to type more, I'm not going to kick up a fuss about \dconf.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company







Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Jonathan S. Katz

On 4/7/22 12:22 PM, Tom Lane wrote:

Justin Pryzby  writes:

SHOW and current_setting() translate to human units, which is particularly
useful for some settings, like those with units of 8k pages.
Is it better to use that "cooked" version for display in the backslash command
instead of the raw view from pg_settings ?


Oh, that's a good idea --- lets us drop the units column entirely.


+1


The attached revision does that and moves the "type" column to
secondary status, as discussed upthread.  I also added docs and
simple regression tests, and fixed two problems that were preventing
completion of custom (qualified) GUC names (we need to use the
VERBATIM option for those queries).  There remains the issue that
tab completion for GUC names ought to be case-insensitive, but
that's a pre-existing bug in tab-complete.c's other GUC name
completions too; I'll tackle it later.

As for the name, \dconf has a slight plurality in votes so far,
so I'm sticking with that.

I think this is ready to go unless someone has a significantly
better idea.


I ran the equivalent SQL locally and it LGTM. Docs read well to me. Code 
looks as good as it can to me.


+1

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Tom Lane
Mark Dilger  writes:
> We have too many synonyms for configuration parameters.  "config", "guc", 
> "parameter", and "setting" are already in use.  I thought we agreed on the 
> other thread that "setting" means the value, and "parameter" is the thing 
> being set.

Right, so the suggestion of \dsetting seems a tad off-kilter.

> It's true that "config" refers to parameters in the name of 
> pg_catalog.set_config, which is a pretty strong precedent, but sadly "config" 
> also refers to configuration files, the build configuration (as in the 
> "pg_config" tool), text search configuration, etc.

I'd also thought briefly about \dpar or \dparam, but I'm not sure that
that's much of an improvement.  \dconf is at least in line with the
docs' terminology of "configuration parameter".  (Note that bare
"parameter" has other meanings too, eg function parameter.)  I wouldn't
fight too hard if people want to lengthen it to \dconfig for consistency
with set_config().

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Tom Lane
Justin Pryzby  writes:
> SHOW and current_setting() translate to human units, which is particularly
> useful for some settings, like those with units of 8k pages.  
> Is it better to use that "cooked" version for display in the backslash command
> instead of the raw view from pg_settings ?

Oh, that's a good idea --- lets us drop the units column entirely.

The attached revision does that and moves the "type" column to
secondary status, as discussed upthread.  I also added docs and
simple regression tests, and fixed two problems that were preventing
completion of custom (qualified) GUC names (we need to use the
VERBATIM option for those queries).  There remains the issue that
tab completion for GUC names ought to be case-insensitive, but
that's a pre-existing bug in tab-complete.c's other GUC name
completions too; I'll tackle it later.

As for the name, \dconf has a slight plurality in votes so far,
so I'm sticking with that.

I think this is ready to go unless someone has a significantly
better idea.

regards, tom lane

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 492a960247..4dc6628add 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2205,7 +2205,7 @@ REVOKE ALL ON accounts FROM PUBLIC;
   PARAMETER
   sA
   none
-  none
+  \dconf+
  
  
   SCHEMA
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index f01adb1fd2..864be97808 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1380,6 +1380,23 @@ testdb=>
   
 
 
+  
+\dconf[+] [ pattern ]
+
+
+Lists server configuration parameters and their values.
+If pattern
+is specified, only parameters whose names match the pattern are
+listed.
+If + is appended to the command name, each
+parameter is listed with its data type, context in which the
+parameter can be set, and access privileges (if non-default access
+privileges have been granted).
+
+
+  
+
+
   
 \dC[+] [ pattern ]
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 079f4a1a76..bbf4a5a44e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -780,7 +780,14 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 success = describeTablespaces(pattern, show_verbose);
 break;
 			case 'c':
-success = listConversions(pattern, show_verbose, show_system);
+if (strncmp(cmd, "dconf", 5) == 0)
+	success = describeConfigurationParameters(pattern,
+			  show_verbose,
+			  show_system);
+else
+	success = listConversions(pattern,
+			  show_verbose,
+			  show_system);
 break;
 			case 'C':
 success = listCasts(pattern, show_verbose);
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 73bbbe2eb4..47590a1302 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -4434,6 +4434,68 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 	return true;
 }
 
+/*
+ * \dconf
+ *
+ * Describes configuration parameters.
+ */
+bool
+describeConfigurationParameters(const char *pattern, bool verbose,
+bool showSystem)
+{
+	PQExpBufferData buf;
+	PGresult   *res;
+	printQueryOpt myopt = pset.popt;
+
+	initPQExpBuffer(&buf);
+	printfPQExpBuffer(&buf,
+	  "SELECT s.name AS \"%s\", "
+	  "pg_catalog.current_setting(s.name) AS \"%s\"",
+	  gettext_noop("Parameter"),
+	  gettext_noop("Value"));
+
+	if (verbose)
+	{
+		appendPQExpBuffer(&buf,
+		  ", s.vartype AS \"%s\", s.context AS \"%s\", ",
+		  gettext_noop("Type"),
+		  gettext_noop("Context"));
+		if (pset.sversion >= 15)
+			printACLColumn(&buf, "p.paracl");
+		else
+			appendPQExpBuffer(&buf, "NULL AS \"%s\"",
+			  gettext_noop("Access privileges"));
+	}
+
+	appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_settings s\n");
+
+	if (verbose && pset.sversion >= 15)
+		appendPQExpBufferStr(&buf,
+			 "  LEFT JOIN pg_catalog.pg_parameter_acl p\n"
+			 "  ON pg_catalog.lower(s.name) = p.parname\n");
+
+	processSQLNamePattern(pset.db, &buf, pattern,
+		  false, false,
+		  NULL, "pg_catalog.lower(s.name)", NULL,
+		  NULL);
+
+	appendPQExpBufferStr(&buf, "ORDER BY 1;");
+
+	res = PSQLexec(buf.data);
+	termPQExpBuffer(&buf);
+	if (!res)
+		return false;
+
+	myopt.nullPrint = NULL;
+	myopt.title = _("List of configuration parameters");
+	myopt.translate_header = true;
+
+	printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+
+	PQclear(res);
+	return true;
+}
+
 /*
  * \dy
  *
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index fd6079679c..4eb2710e27 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -76,6 +76,10 @@ extern bool listDomains(const char *pattern, bool verbose, bool showSystem);
 /* \dc */
 extern

Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Mark Dilger



> On Apr 7, 2022, at 6:21 AM, Andrew Dunstan  wrote:
> 
> \dconf seems fine to me

We have too many synonyms for configuration parameters.  "config", "guc", 
"parameter", and "setting" are already in use.  I thought we agreed on the 
other thread that "setting" means the value, and "parameter" is the thing being 
set.  It's true that "config" refers to parameters in the name of 
pg_catalog.set_config, which is a pretty strong precedent, but sadly "config" 
also refers to configuration files, the build configuration (as in the 
"pg_config" tool), text search configuration, etc.

While grep'ing through doc/src/sgml, I see no instances of "conf" ever 
referring to configuration parameters.  It only ever refers to configuration 
files.  I'd prefer not adding it to the list of synonyms.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company







Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Jonathan S. Katz

On 4/7/22 11:10 AM, David G. Johnston wrote:
On Thu, Apr 7, 2022 at 7:56 AM Tom Lane > wrote:


"Jonathan S. Katz" mailto:jk...@postgresql.org>> writes:

 > Maybe to appeal to all crowds, we say "list configuration parameters
 > (GUCs)"?

I'm in the camp that says that GUC is not an acronym we wish to expose
to end users.


I am too.


WFM.

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Justin Pryzby
On Wed, Apr 06, 2022 at 11:02:54PM -0400, Tom Lane wrote:
> "Jonathan S. Katz"  writes:
> > +1 for \dconf
> 
> Here's a draft patch using \dconf.  No tests or docs yet.

The patch as written is a thin layer around pg_settings.

SHOW and current_setting() translate to human units, which is particularly
useful for some settings, like those with units of 8k pages.  

Is it better to use that "cooked" version for display in the backslash command
instead of the raw view from pg_settings ?

Otherwise, I see myself first using tab completion or, failing that,
SELECT * FROM pg_settings WHERE name~'something', followed by SHOW, to
avoid messing up counting digits, multiplication or unit conversions.

> + printfPQExpBuffer(&buf,
> +   "SELECT s.name AS \"%s\", s.setting 
> AS \"%s\", "
> +   "s.unit AS \"%s\", s.vartype AS 
> \"%s\"",
> +   gettext_noop("Parameter"),
> +   gettext_noop("Setting"),
> +   gettext_noop("Unit"),
> +   gettext_noop("Type"));
> +

> + appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_settings s\n");




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Josef Šimánek
st 6. 4. 2022 v 19:49 odesílatel Tom Lane  napsal:
>
> It's not difficult to get psql to show you the current value
> of a single GUC --- "SHOW" does that fine, and it has tab
> completion support for the GUC name.  However, I very often
> find myself resorting to the much more tedious
>
> select * from pg_settings where name like '%foo%';
>
> when I want to see some related parameters, or when I'm a bit
> fuzzy on the exact name of the parameter.  Not only is this
> a lot of typing, but unless I'm willing to type even more to
> avoid using "*", I'll get a wall of mostly unreadable text,
> because pg_settings is far too wide and cluttered with
> low-grade information.
>
> In the discussion about adding privileges for GUCs [1], there
> was a proposal to add a new psql backslash command to show GUCs,
> which could reduce this problem to something like
>
> \dcp *foo*
>
> (The version proposed there was not actually useful for this
> purpose because it was too narrowly focused on GUCs with
> privileges, but that's easily fixed.)
>
> So does anyone else like this idea?

I like this idea. Also I'm interested in contributing this. Feel free
to ping me if welcomed, I can try to prepare at least the initial
patch. Currently it seems the discussion is related mostly to the
command name, which can be changed at any time.

> In detail, I'd imagine this command showing the name, setting, unit,
> and vartype fields of pg_setting by default, and if you add "+"
> then it should add the context field, as well as applicable
> privileges when server version >= 15.  However, there's plenty
> of room for bikeshedding that list of columns, not to mention
> the precise name of the command.  (I'm not that thrilled with
> "\dcp" myself, as it looks like it might be a sub-form of "\dc".)
> So I thought I'd solicit comments before working on a patch
> not after.
>
> I view this as being at least in part mop-up for commit a0ffa885e,
> especially since a form of this was discussed in that thread.
> So I don't think it'd be unreasonable to push into v15, even
> though it's surely a new feature.
>
> regards, tom lane
>
> [1] 
> https://www.postgresql.org/message-id/flat/3d691e20-c1d5-4b80-8ba5-6beb63af3...@enterprisedb.com
>
>




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread David G. Johnston
On Thu, Apr 7, 2022 at 7:56 AM Tom Lane  wrote:

> "Jonathan S. Katz"  writes:
>
>
> Maybe to appeal to all crowds, we say "list configuration parameters
> > (GUCs)"?
>
> I'm in the camp that says that GUC is not an acronym we wish to expose
> to end users.
>
>
I am too.  In any case, either go all-in with GUC (i.e., \dG or \dguc) or
pretend it doesn't exist - an in-between position is unappealing.

David J.


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Tom Lane
"Jonathan S. Katz"  writes:
> On 4/7/22 8:36 AM, Joe Conway wrote:
>> I will say that I care about context far more often than unit or type 
>> though, so from my point of view I would switch them around with respect 
>> to which is only shown with verbose.

> I disagree somewhat -- I agree the context should be in the regular 
> view, but unit and type are also important. If I had to choose to drop 
> one, I'd choose type as it could be inferred, but I would say better to 
> keep them all.

Given the new ability to grant privileges on GUCs, context alone is not
sufficient to know when something can be set.  So the context and the
privileges seem like they should go together, and that's why I have them
both under "+".

I can see the argument for relegating type to the "+" format, in hopes of
keeping the default display narrow enough for ordinary terminal windows.

> A couple of minor things:
>   +   appendPQExpBufferStr(&buf, "ORDER BY 1;");
> I don't know how much we do positional ordering in our queries, but it 
> may be better to explicitly order by "s.name".

"ORDER BY n" seems to be the de facto standard in describe.c.  Perhaps
there's an argument for changing it throughout that file, but I don't
think this one function should be out of step with the rest.

> I don't know if we want to throw a "LOWER(s.name)" on at least the 
> ordering, given we allow for "SHOW" itself to load these case-insensitively.

Yeah, I went back and forth on that myself --- I was looking at the
example of DateStyle, and noticing that although you see it in mixed
case in the command's output, tab completion isn't happy unless you
enter it in lower case (ie, date works, Date not so much).
Forcibly lowercasing the command output would fix that inconsistency,
but on the other hand it introduces an inconsistency with what the
pg_settings view shows.  Not sure what's the least bad.  We might be
able to fix the tab completion behavior, if we don't mind complicating
tab-complete.c even more; so probably that's the thing to look at
before changing the output.

> Maybe to appeal to all crowds, we say "list configuration parameters 
> (GUCs)"?

I'm in the camp that says that GUC is not an acronym we wish to expose
to end users.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Jonathan S. Katz

On 4/7/22 8:36 AM, Joe Conway wrote:

On 4/6/22 23:02, Tom Lane wrote:

"Jonathan S. Katz"  writes:

+1 for \dconf


Here's a draft patch using \dconf.  No tests or docs yet.


WFM -- using some form of \d makes more sense than 
\s, and I can't think of anything better that \dconf.


I will say that I care about context far more often than unit or type 
though, so from my point of view I would switch them around with respect 
to which is only shown with verbose.


I disagree somewhat -- I agree the context should be in the regular 
view, but unit and type are also important. If I had to choose to drop 
one, I'd choose type as it could be inferred, but I would say better to 
keep them all.


The downside is that by including context, the standard list appears to 
push past my 99px width terminal in non-enhanced view, but that may be OK.



A couple of minor things:

+   appendPQExpBufferStr(&buf, "ORDER BY 1;");

I don't know how much we do positional ordering in our queries, but it 
may be better to explicitly order by "s.name". I doubt this column name 
is likely to change, and if for some reason someone shuffles the output 
order of \dconf, it makes it less likely to break someone's view.


I did not test this via an extension, but we do allow for mixed case in 
custom GUCs:


postgres=# SHOW jkatz.test;
 JKATZ.test

 abc

I don't know if we want to throw a "LOWER(s.name)" on at least the 
ordering, given we allow for "SHOW" itself to load these case-insensitively.


	+	fprintf(output, _("  \\dconf[+] [PATTERN]list configuration 
parameters\n"));


Maybe to appeal to all crowds, we say "list configuration parameters 
(GUCs)"?


Thanks,

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Andrew Dunstan


On 4/6/22 23:25, David Rowley wrote:
> I also find myself querying pg_settings all too often. More typing
> than I'd like.
>
> On Thu, 7 Apr 2022 at 06:40, Tom Lane  wrote:
>> I do agree that \show might be a bad choice, the reason being that
>> the adjacent \set command is for psql variables not GUCs; if we
>> had a \show I'd sort of expect it to be a variant spelling of
>> "\echo :variable".
> I also think \show is not a great choice. I'd rather see us follow the
> \d pattern for showing information about objects in the database.
>
>> "\sc" isn't awful perhaps.
> I think \dG is pretty good. G for GUC.
>


-1 on anything that is based on "GUC", an ancient and now largely
irrelevant acronym. How many developers, let alone users, know what it
stands for?


\dconf seems fine to me


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com





Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Joe Conway

On 4/6/22 23:02, Tom Lane wrote:

"Jonathan S. Katz"  writes:

+1 for \dconf


Here's a draft patch using \dconf.  No tests or docs yet.


WFM -- using some form of \d makes more sense than 
\s, and I can't think of anything better that \dconf.


I will say that I care about context far more often than unit or type 
though, so from my point of view I would switch them around with respect 
to which is only shown with verbose.


Joe




Re: How about a psql backslash command to show GUCs?

2022-04-07 Thread Pavel Luzanov

On 06.04.2022 20:48, Tom Lane wrote:

However, I very often
find myself resorting to the much more tedious

select * from pg_settings where name like '%foo%';



In the discussion about adding privileges for GUCs [1], there
was a proposal to add a new psql backslash command to show GUCs,
which could reduce this problem to something like

\dcp *foo*


+1, great idea.

Right now I use the psql :show variable in my .psqlrc for this purpose:

=# \echo :show
SELECT name, current_setting(name) AS value, context FROM pg_settings\g 
(format=wrapped columns=100) | grep


=# :show autovacuum
 autovacuum | 
on    | sighup
 autovacuum_analyze_scale_factor    | 
0.1   | sighup
 autovacuum_analyze_threshold   | 
50    | sighup
 autovacuum_freeze_max_age  | 
2 | postmaster
 autovacuum_max_workers | 
3 | postmaster
 autovacuum_multixact_freeze_max_age    | 
4 | postmaster
 autovacuum_naptime | 
1min  | sighup
 autovacuum_vacuum_cost_delay   | 
2ms   | sighup
 autovacuum_vacuum_cost_limit   | 
-1    | sighup
 autovacuum_vacuum_scale_factor | 
0.2   | sighup
 autovacuum_vacuum_threshold    | 
50    | sighup
 autovacuum_work_mem    | 
-1    | sighup
 log_autovacuum_min_duration    | 
-1    | sighup


As for the name, I can't think of a better candidate. Any of the 
previously suggested list of \dconf, \dguc, \dG, \dcp is fine.


--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
The Russian Postgres Company





Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread David Rowley
I also find myself querying pg_settings all too often. More typing
than I'd like.

On Thu, 7 Apr 2022 at 06:40, Tom Lane  wrote:
> I do agree that \show might be a bad choice, the reason being that
> the adjacent \set command is for psql variables not GUCs; if we
> had a \show I'd sort of expect it to be a variant spelling of
> "\echo :variable".

I also think \show is not a great choice. I'd rather see us follow the
\d pattern for showing information about objects in the database.

> "\sc" isn't awful perhaps.

I think \dG is pretty good. G for GUC.

David




Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Tom Lane
"Jonathan S. Katz"  writes:
> +1 for \dconf

Here's a draft patch using \dconf.  No tests or docs yet.

regards, tom lane

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 079f4a1a76..bbf4a5a44e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -780,7 +780,14 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 success = describeTablespaces(pattern, show_verbose);
 break;
 			case 'c':
-success = listConversions(pattern, show_verbose, show_system);
+if (strncmp(cmd, "dconf", 5) == 0)
+	success = describeConfigurationParameters(pattern,
+			  show_verbose,
+			  show_system);
+else
+	success = listConversions(pattern,
+			  show_verbose,
+			  show_system);
 break;
 			case 'C':
 success = listCasts(pattern, show_verbose);
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 4dddf08789..e55cfcb2f3 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -4430,6 +4430,68 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
 	return true;
 }
 
+/*
+ * \dconf
+ *
+ * Describes configuration parameters.
+ */
+bool
+describeConfigurationParameters(const char *pattern, bool verbose,
+bool showSystem)
+{
+	PQExpBufferData buf;
+	PGresult   *res;
+	printQueryOpt myopt = pset.popt;
+
+	initPQExpBuffer(&buf);
+	printfPQExpBuffer(&buf,
+	  "SELECT s.name AS \"%s\", s.setting AS \"%s\", "
+	  "s.unit AS \"%s\", s.vartype AS \"%s\"",
+	  gettext_noop("Parameter"),
+	  gettext_noop("Setting"),
+	  gettext_noop("Unit"),
+	  gettext_noop("Type"));
+
+	if (verbose)
+	{
+		appendPQExpBuffer(&buf, ", s.context AS \"%s\", ",
+		  gettext_noop("Context"));
+		if (pset.sversion >= 15)
+			printACLColumn(&buf, "p.paracl");
+		else
+			appendPQExpBuffer(&buf, "NULL AS \"%s\"",
+			  gettext_noop("Access privileges"));
+	}
+
+	appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_settings s\n");
+
+	if (verbose && pset.sversion >= 15)
+		appendPQExpBufferStr(&buf,
+			 "  LEFT JOIN pg_catalog.pg_parameter_acl p\n"
+			 "  ON pg_catalog.lower(s.name) = p.parname\n");
+
+	processSQLNamePattern(pset.db, &buf, pattern,
+		  false, false,
+		  NULL, "pg_catalog.lower(s.name)", NULL,
+		  NULL);
+
+	appendPQExpBufferStr(&buf, "ORDER BY 1;");
+
+	res = PSQLexec(buf.data);
+	termPQExpBuffer(&buf);
+	if (!res)
+		return false;
+
+	myopt.nullPrint = NULL;
+	myopt.title = _("List of configuration parameters");
+	myopt.translate_header = true;
+
+	printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+
+	PQclear(res);
+	return true;
+}
+
 /*
  * \dy
  *
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index fd6079679c..4eb2710e27 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -76,6 +76,10 @@ extern bool listDomains(const char *pattern, bool verbose, bool showSystem);
 /* \dc */
 extern bool listConversions(const char *pattern, bool verbose, bool showSystem);
 
+/* \dconf */
+extern bool describeConfigurationParameters(const char *pattern, bool verbose,
+			bool showSystem);
+
 /* \dC */
 extern bool listCasts(const char *pattern, bool verbose);
 
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index b3971bce64..eff9d1a16e 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -166,7 +166,7 @@ slashUsage(unsigned short int pager)
 	 * Use "psql --help=commands | wc" to count correctly.  It's okay to count
 	 * the USE_READLINE line even in builds without that.
 	 */
-	output = PageOutput(137, pager ? &(pset.popt.topt) : NULL);
+	output = PageOutput(138, pager ? &(pset.popt.topt) : NULL);
 
 	fprintf(output, _("General\n"));
 	fprintf(output, _("  \\copyright show PostgreSQL usage and distribution terms\n"));
@@ -231,6 +231,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dAp[+] [AMPTRN [OPFPTRN]]   list support functions of operator families\n"));
 	fprintf(output, _("  \\db[+]  [PATTERN]  list tablespaces\n"));
 	fprintf(output, _("  \\dc[S+] [PATTERN]  list conversions\n"));
+	fprintf(output, _("  \\dconf[+] [PATTERN]list configuration parameters\n"));
 	fprintf(output, _("  \\dC[+]  [PATTERN]  list casts\n"));
 	fprintf(output, _("  \\dd[S]  [PATTERN]  show object descriptions not displayed elsewhere\n"));
 	fprintf(output, _("  \\dD[S+] [PATTERN]  list domains\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 32d0b4855f..015037d626 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1690,7 +1690,7 @@ psql_completion(const char *text, int start, int end)
 		"\\connect", "\\conninfo", "\\C", "\\cd", "\\copy",
 		"\\copyright", "\\crosstabview",
 		"\\d", "\\da", "\\dA", "\\dAc", "\\dAf", "\\dAo", "\\dAp",
-		"\\db", "\\dc", "\\dC", "\\dd", "\\ddp", "\\dD",
+		"\\db", "\\dc", "\\dconf", "\\dC", "

Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Jonathan S. Katz

On 4/6/22 9:18 PM, David G. Johnston wrote:
On Wed, Apr 6, 2022 at 6:16 PM Tom Lane 


I agree that \d-something makes the most sense from a functionality
standpoint.  But I don't want to make the name too long, even if we
do have tab completion to help.

\dconf maybe?


I don't have a strong preference, but just tossing it out there; maybe 
embrace the novelty of GUC?


\dguc


+1 for \dconf

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread David G. Johnston
On Wed, Apr 6, 2022 at 6:16 PM Tom Lane  wrote:

> "Jonathan S. Katz"  writes:
> > I am a bit torn between "\dcp" (or \dsetting / \dconfig? we don't
> > necessarily need for it to be super short) and "\sc". Certainly with
> > pattern matching the interface for the "\d" commands would fit that
> > pattern. "\sc" would make sense for a thorough introspection of what is
> > in the GUC. That said, we get that with SHOW today.
>
> > So I'm leaning towards something in the "\d" family.
>
> I agree that \d-something makes the most sense from a functionality
> standpoint.  But I don't want to make the name too long, even if we
> do have tab completion to help.
>
> \dconf maybe?
>
>
I don't have a strong preference, but just tossing it out there; maybe
embrace the novelty of GUC?

\dguc

David J.


Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Tom Lane
"Jonathan S. Katz"  writes:
> I am a bit torn between "\dcp" (or \dsetting / \dconfig? we don't 
> necessarily need for it to be super short) and "\sc". Certainly with 
> pattern matching the interface for the "\d" commands would fit that 
> pattern. "\sc" would make sense for a thorough introspection of what is 
> in the GUC. That said, we get that with SHOW today.

> So I'm leaning towards something in the "\d" family.

I agree that \d-something makes the most sense from a functionality
standpoint.  But I don't want to make the name too long, even if we
do have tab completion to help.

\dconf maybe?

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Mark Dilger



> On Apr 6, 2022, at 2:34 PM, Jonathan S. Katz  wrote:
> 
> "\sc" would make sense

I originally wrote the command as \dcp (describe configuration parameter) 
because \dp (describe parameter) wasn't available.  The thing we're showing is 
a "parameter", not a "config".  If we're going to use a single letter, I'd 
prefer /p/.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company







Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Jonathan S. Katz

On 4/6/22 2:40 PM, Tom Lane wrote:

Joe Conway  writes:

No as sure about \show though. That seems like it could be confused with
showing other stuff. Maybe consistent with \sf[+] and \sv[+] we could
add \sc[+]?


Hmm ... my first reaction to that was "no, it should be \sp for
'parameter'".  But with the neighboring \sf for 'function', it'd
be easy to think that maybe 'p' means 'procedure'.

I do agree that \show might be a bad choice, the reason being that
the adjacent \set command is for psql variables not GUCs; if we
had a \show I'd sort of expect it to be a variant spelling of
"\echo :variable".

"\sc" isn't awful perhaps.

Ah, naming ... the hardest problem in computer science.


(but the easiest thing to have an opinion on ;)

+1 on the feature proposal.

I am a bit torn between "\dcp" (or \dsetting / \dconfig? we don't 
necessarily need for it to be super short) and "\sc". Certainly with 
pattern matching the interface for the "\d" commands would fit that 
pattern. "\sc" would make sense for a thorough introspection of what is 
in the GUC. That said, we get that with SHOW today.


So I'm leaning towards something in the "\d" family.

Thanks,

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Greg Stark
On Wed, 6 Apr 2022 at 13:50, Tom Lane  wrote:
>
> when I want to see some related parameters, or when I'm a bit
> fuzzy on the exact name of the parameter.  Not only is this
> a lot of typing, but unless I'm willing to type even more to
> avoid using "*", I'll get a wall of mostly unreadable text,
> because pg_settings is far too wide and cluttered with
> low-grade information.

I may be suffering from some form of the Mandela Effect but I have a
distinct memory that once upon a time SHOW actually took patterns like
\d does. I keep trying it, forgetting that it doesn't actually work.
I'm guessing my brain is generalizing and assuming SHOW fits into the
same pattern as \d commands and just keeps doing it even after I've
learned repeatedly that it doesn't work.

Personally I would like to just make the world match the way my brain
thinks it is and make this just work:

SHOW enable_*

I don't see any value in allowing * or ? in GUC names so I don't even
see a downside to this. It would be way easier to discover than
another \ command

-- 
greg




Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Tom Lane
Joe Conway  writes:
> No as sure about \show though. That seems like it could be confused with 
> showing other stuff. Maybe consistent with \sf[+] and \sv[+] we could 
> add \sc[+]?

Hmm ... my first reaction to that was "no, it should be \sp for
'parameter'".  But with the neighboring \sf for 'function', it'd
be easy to think that maybe 'p' means 'procedure'.

I do agree that \show might be a bad choice, the reason being that
the adjacent \set command is for psql variables not GUCs; if we
had a \show I'd sort of expect it to be a variant spelling of
"\echo :variable".

"\sc" isn't awful perhaps.

Ah, naming ... the hardest problem in computer science.

regards, tom lane




Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Joe Conway

On 4/6/22 13:58, Alvaro Herrera wrote:

On 2022-Apr-06, Tom Lane wrote:


However, I very often find myself resorting to the much more tedious

select * from pg_settings where name like '%foo%';

when I want to see some related parameters, or when I'm a bit
fuzzy on the exact name of the parameter.


Been there many times, so +1 for the general idea.


In the discussion about adding privileges for GUCs [1], there
was a proposal to add a new psql backslash command to show GUCs,
which could reduce this problem to something like

\dcp *foo*


+1.  As for command name, I like \show as proposed by Pavel.


+1

I'd love something for the same reasons.

No as sure about \show though. That seems like it could be confused with 
showing other stuff. Maybe consistent with \sf[+] and \sv[+] we could 
add \sc[+]?


Joe




Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Mark Dilger



> On Apr 6, 2022, at 10:48 AM, Tom Lane  wrote:
> 
> So does anyone else like this idea?

Privileges on targets other than parameters have a \d command to show the 
privileges, as listed in doc/src/sgml/ddl.sgml.  There isn't an obvious reason 
for omitting parameters from the list so covered.

One of the ideas that got punted in the recent commit was to make it possible 
to revoke SET on a USERSET guc.  For example, it might be nice to REVOKE SET ON 
PARAMETER work_mem FROM PUBLIC.  That can't be done now, but for some select 
parameters, we might implement that in the future by promoting them to SUSET 
with a default GRANT SET...TO PUBLIC.  When connecting to databases of 
different postgres versions (albeit only those version 15 and above), it'd be 
nice to quickly see what context (USERSET vs. SUSET) is assigned to the 
parameter, and whether the GRANT has been revoked.

So yes, +1 from me.
 
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company







Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Alvaro Herrera
On 2022-Apr-06, Tom Lane wrote:

> However, I very often find myself resorting to the much more tedious
> 
> select * from pg_settings where name like '%foo%';
> 
> when I want to see some related parameters, or when I'm a bit
> fuzzy on the exact name of the parameter.

Been there many times, so +1 for the general idea.

> In the discussion about adding privileges for GUCs [1], there
> was a proposal to add a new psql backslash command to show GUCs,
> which could reduce this problem to something like
> 
> \dcp *foo*

+1.  As for command name, I like \show as proposed by Pavel.

-- 
Álvaro HerreraBreisgau, Deutschland  —  https://www.EnterpriseDB.com/




Re: How about a psql backslash command to show GUCs?

2022-04-06 Thread Pavel Stehule
hi

st 6. 4. 2022 v 19:49 odesílatel Tom Lane  napsal:

> It's not difficult to get psql to show you the current value
> of a single GUC --- "SHOW" does that fine, and it has tab
> completion support for the GUC name.  However, I very often
> find myself resorting to the much more tedious
>
> select * from pg_settings where name like '%foo%';
>
> when I want to see some related parameters, or when I'm a bit
> fuzzy on the exact name of the parameter.  Not only is this
> a lot of typing, but unless I'm willing to type even more to
> avoid using "*", I'll get a wall of mostly unreadable text,
> because pg_settings is far too wide and cluttered with
> low-grade information.
>
> In the discussion about adding privileges for GUCs [1], there
> was a proposal to add a new psql backslash command to show GUCs,
> which could reduce this problem to something like
>
> \dcp *foo*
>

It can be a good idea. I am not sure about \dcp. maybe \show can be better?

Regards

Pavel


>
> (The version proposed there was not actually useful for this
> purpose because it was too narrowly focused on GUCs with
> privileges, but that's easily fixed.)
>
> So does anyone else like this idea?
>
> In detail, I'd imagine this command showing the name, setting, unit,
> and vartype fields of pg_setting by default, and if you add "+"
> then it should add the context field, as well as applicable
> privileges when server version >= 15.  However, there's plenty
> of room for bikeshedding that list of columns, not to mention
> the precise name of the command.  (I'm not that thrilled with
> "\dcp" myself, as it looks like it might be a sub-form of "\dc".)
> So I thought I'd solicit comments before working on a patch
> not after.
>
> I view this as being at least in part mop-up for commit a0ffa885e,
> especially since a form of this was discussed in that thread.
> So I don't think it'd be unreasonable to push into v15, even
> though it's surely a new feature.
>
> regards, tom lane
>
> [1]
> https://www.postgresql.org/message-id/flat/3d691e20-c1d5-4b80-8ba5-6beb63af3...@enterprisedb.com
>
>
>