Обсуждение: consider including server_version in explain(settings)

Поиск
Список
Период
Сортировка

consider including server_version in explain(settings)

От
Justin Pryzby
Дата:
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 */



Re: consider including server_version in explain(settings)

От
Tom Lane
Дата:
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