consider including server_version in explain(settings)

Started by Justin Pryzbyover 6 years ago2 messageshackers
Jump to latest
#1Justin Pryzby
pryzby@telsasoft.com

explain(SETTINGS) was implemented to show relevant settings for which an odd
value could affect a query but could be forgotten during troubleshooting.

This is a "concept" patch to show the version, which is frequently requested on
-performance list and other support requests. If someone sends
explain(settings), they don't need to also (remember to) send the version..

postgres=# explain(settings)SELECT;
Result (cost=0.00..0.01 rows=1 width=0)
Settings: server_version_num = '130000', work_mem = '128MB'

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 85ca2b3..2edc83c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3143,7 +3143,7 @@ static struct config_int ConfigureNamesInt[] =
 		{"server_version_num", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows the server version as an integer."),
 			NULL,
-			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_EXPLAIN
 		},
 		&server_version_num,
 		PG_VERSION_NUM, PG_VERSION_NUM, PG_VERSION_NUM,
@@ -8955,7 +8955,7 @@ get_explain_guc_options(int *num)
 		}
 		/* skip GUC variables that match the built-in default */
-		if (!modified)
+		if (!modified && strcmp(conf->name, "server_version_num"))
 			continue;

/* assign to the values array */

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Justin Pryzby (#1)
Re: consider including server_version in explain(settings)

Justin Pryzby <pryzby@telsasoft.com> writes:

This is a "concept" patch to show the version, which is frequently requested on
-performance list and other support requests. If someone sends
explain(settings), they don't need to also (remember to) send the version..

I'm not really on board with the proposal at all here; I think it'll
be useless clutter most of the time. I do not agree with the position
that the only use-case for explain(settings) is performance trouble
reports. Moreover, if we start including fixed settings then where
do we stop? People might also want "pg_config" output for example,
and that's surely not reasonable to include in EXPLAIN.

Independently of that, however:

/* skip GUC variables that match the built-in default */
-		if (!modified)
+		if (!modified && strcmp(conf->name, "server_version_num"))
continue;

This is both horribly contorted logic (it could at least do with a
comment) and against project coding conventions (do not use the result
of strcmp() as if it were a boolean).

regards, tom lane