From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: explain plans with information about (modified) gucs
Date: Sat, 15 Dec 2018 16:44:48 +0100
Message-ID: <631ae5c8-5581-6c5e-382c-2dcdb2e5c956@2ndquadrant.com> (raw)
In-Reply-To: <c1282df1-43a8-0854-dfee-790d7dd9c93a@2ndquadrant.com>
References: <e1791b4c-df9c-be02-edc5-7c8874944be0@2ndquadrant.com>
<10561.1544800904@sss.pgh.pa.us>
<c1282df1-43a8-0854-dfee-790d7dd9c93a@2ndquadrant.com>
On 12/14/18 4:32 PM, Tomas Vondra wrote:
>
>
> On 12/14/18 4:21 PM, Tom Lane wrote:
>> Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>>> ... I propose to extend EXPLAIN output with an additional option, which
>>> would include information about modified GUCs in the execution plan
>>> (disabled by default, of course):
>>
>> I'm a bit suspicious about whether this'll have any actual value,
>> if it's disabled by default (which I agree it needs to be, if only for
>> compatibility reasons). The problem you're trying to solve is basically
>> "I forgot that this might have an effect", but stuff that isn't shown
>> by default will not help you un-forget. It certainly won't fix the
>> form of the problem that I run into, which is people sending in EXPLAIN
>> plans and not mentioning their weird local settings.
>>
>
> Not quite.
>
> I agree we'll still have to deal with plans from users without this
> info, but it's easier to ask for explain with this extra option (just
> like we regularly ask for explain analyze instead of just plain
> explain). I'd expect the output to be more complete than trying to
> figure out which of the GUCs might have effect / been modified here.
>
> But more importantly - my personal primary use case here is explains
> from application connections generated using auto_explain, with some
> application-level GUC magic. And there I can easily tweak auto_explain
> config to do (auto_explain.log_gucs = true) of course.
>
>>> We certainly don't want to include all GUCs, so the question is how to
>>> decide which GUCs are interesting. The simplest approach would be to
>>> look for GUCs that changed in the session (source == PGC_S_SESSION), but
>>> that does not help with SET SESSION AUTHORIZATION / ROLE cases. So we
>>> probably want (source > PGC_S_ARGV), but probably not PGC_S_OVERRIDE
>>> because that includes irrelevant options like wal_buffers etc.
>>
>> Don't you want to show anything that's not the built-in default?
>> (I agree OVERRIDE could be excluded, but that's irrelevant for query
>> tuning parameters.) Just because somebody injected a damfool setting
>> of, say, random_page_cost via the postmaster command line or
>> environment settings doesn't make it not damfool :-(
>>
>
> Probably. My assumption here was that I can do
>
> select * from pg_settings
>
> and then combine it with whatever is included in the plan. But you're
> right comparing it with the built-in default may be a better option.
>
FWIW here is a v3 of the patch, using the built-in default, and fixing a
silly thinko resulting in the code not being executed from auto_explain.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
[text/x-patch] explain-with-gucs-v3.patch (7.5K, ../631ae5c8-5581-6c5e-382c-2dcdb2e5c956@2ndquadrant.com/2-explain-with-gucs-v3.patch)
download | inline diff:diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.cindex 646cd0d42c..ec44c59b7f 100644--- a/contrib/auto_explain/auto_explain.c+++ b/contrib/auto_explain/auto_explain.c@@ -28,6 +28,7 @@ static bool auto_explain_log_verbose = false;
static bool auto_explain_log_buffers = false;
static bool auto_explain_log_triggers = false;
static bool auto_explain_log_timing = true;
+static bool auto_explain_log_gucs = false;
static int auto_explain_log_format = EXPLAIN_FORMAT_TEXT;
static int auto_explain_log_level = LOG;
static bool auto_explain_log_nested_statements = false;
@@ -112,6 +113,17 @@ _PG_init(void)
NULL,
NULL);
+ DefineCustomBoolVariable("auto_explain.log_gucs",+ "Print modified GUC values.",+ NULL,+ &auto_explain_log_gucs,+ false,+ PGC_SUSET,+ 0,+ NULL,+ NULL,+ NULL);+
DefineCustomBoolVariable("auto_explain.log_verbose",
"Use EXPLAIN VERBOSE for plan logging.",
NULL,
@@ -356,6 +368,7 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
es->timing = (es->analyze && auto_explain_log_timing);
es->summary = es->analyze;
es->format = auto_explain_log_format;
+ es->gucs = auto_explain_log_gucs;
ExplainBeginOutput(es);
ExplainQueryText(es, queryDesc);
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgmlindex 120b168d45..852c69b7bb 100644--- a/doc/src/sgml/auto-explain.sgml+++ b/doc/src/sgml/auto-explain.sgml@@ -169,6 +169,23 @@ LOAD 'auto_explain';
</listitem>
</varlistentry>
+ <varlistentry>+ <term>+ <varname>auto_explain.log_gucs</varname> (<type>boolean</type>)+ <indexterm>+ <primary><varname>auto_explain.log_gucs</varname> configuration parameter</primary>+ </indexterm>+ </term>+ <listitem>+ <para>+ <varname>auto_explain.log_gucs</varname> controls whether information+ about modified configuration options are logged with the execution+ plan. Only options modified at the database, user, client or session+ level are considered modified. This parameter is off by default.+ </para>+ </listitem>+ </varlistentry>+
<varlistentry>
<term>
<varname>auto_explain.log_format</varname> (<type>enum</type>)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.cindex de09ded65b..2e92690453 100644--- a/src/backend/commands/explain.c+++ b/src/backend/commands/explain.c@@ -31,6 +31,7 @@
#include "storage/bufmgr.h"
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
+#include "utils/guc_tables.h"
#include "utils/json.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
@@ -164,6 +165,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
es->costs = defGetBoolean(opt);
else if (strcmp(opt->defname, "buffers") == 0)
es->buffers = defGetBoolean(opt);
+ else if (strcmp(opt->defname, "gucs") == 0)+ es->gucs = defGetBoolean(opt);
else if (strcmp(opt->defname, "timing") == 0)
{
timing_set = true;
@@ -635,6 +638,41 @@ ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc)
if (IsA(ps, GatherState) &&((Gather *) ps->plan)->invisible)
ps = outerPlanState(ps);
ExplainNode(ps, NIL, NULL, NULL, es);
++ /*+ * If requested, include information about GUC parameters that don't+ * match the built-in defaults.+ */+ if (es->gucs)+ {+ int i;+ int num;+ StringInfoData str;+ struct config_generic **gucs;++ gucs = get_modified_guc_options(&num);++ for (i = 0; i < num; i++)+ {+ char *setting;+ struct config_generic *conf = gucs[i];++ if (i == 0)+ initStringInfo(&str);+ else+ appendStringInfoString(&str, ", ");++ setting = GetConfigOptionByName(conf->name, NULL, true);++ if (setting)+ appendStringInfo(&str, "%s = '%s'", conf->name, setting);+ else+ appendStringInfo(&str, "%s = NULL", conf->name);+ }++ if (num > 0)+ ExplainPropertyText("GUCs", str.data, es);+ }
}
/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex 6fe1939881..8223c041f7 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -8556,6 +8556,90 @@ ShowAllGUCConfig(DestReceiver *dest)
end_tup_output(tstate);
}
+struct config_generic **+get_modified_guc_options(int *num)+{+ int i;+ struct config_generic **result;++ *num = 0;+ result = palloc(sizeof(struct config_generic *) * num_guc_variables);++ for (i = 0; i < num_guc_variables; i++)+ {+ bool modified;+ struct config_generic *conf = guc_variables[i];++ /* return only options visible to the user */+ if ((conf->flags & GUC_NO_SHOW_ALL) ||+ ((conf->flags & GUC_SUPERUSER_ONLY) &&+ !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)))+ continue;++ /* only parameters related to query tuning */+ if ((conf->group != QUERY_TUNING) &&+ (conf->group != QUERY_TUNING_METHOD) &&+ (conf->group != QUERY_TUNING_COST) &&+ (conf->group != QUERY_TUNING_GEQO) &&+ (conf->group != QUERY_TUNING_OTHER))+ continue;++ /* return only options that were modified (w.r.t. config file) */+ modified = false;++ switch (conf->vartype)+ {+ case PGC_BOOL:+ {+ struct config_bool *lconf = (struct config_bool *) conf;+ modified = (lconf->boot_val != *(lconf->variable));+ }+ break;++ case PGC_INT:+ {+ struct config_int *lconf = (struct config_int *) conf;+ modified = (lconf->boot_val != *(lconf->variable));+ }+ break;++ case PGC_REAL:+ {+ struct config_real *lconf = (struct config_real *) conf;+ modified = (lconf->boot_val != *(lconf->variable));+ }+ break;++ case PGC_STRING:+ {+ struct config_string *lconf = (struct config_string *) conf;+ modified = (strcmp(lconf->boot_val, *(lconf->variable)) != 0);+ }+ break;++ case PGC_ENUM:+ {+ struct config_enum *lconf = (struct config_enum *) conf;+ modified = (lconf->boot_val != *(lconf->variable));+ }+ break;++ default:+ elog(ERROR, "unexcpected GUC type: %d", conf->vartype);+ }++ /* skip GUC variables that match the built-in default */+ if (!modified)+ continue;++ /* assign to the values array */+ result[*num] = conf;+ *num = *num + 1;+ }++ return result;+}+
/*
* Return GUC variable value by name; optionally return canonical form of
* name. If the GUC is unset, then throw an error unless missing_ok is true,
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.hindex d3f70fda08..05afca22aa 100644--- a/src/include/commands/explain.h+++ b/src/include/commands/explain.h@@ -35,6 +35,7 @@ typedef struct ExplainState
bool buffers; /* print buffer usage */
bool timing; /* print detailed node timing */
bool summary; /* print total planning and execution timing */
+ bool gucs; /* print modified GUCs */
ExplainFormat format; /* output format */
/* state for output formatting --- not reset for each new plan tree */
int indent; /* current indentation level */
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.hindex 6f9fdb6a5f..4942e192d6 100644--- a/src/include/utils/guc_tables.h+++ b/src/include/utils/guc_tables.h@@ -267,5 +267,6 @@ extern void build_guc_variables(void);
extern const char *config_enum_lookup_by_value(struct config_enum *record, int val);
extern bool config_enum_lookup_by_name(struct config_enum *record,
const char *value, int *retval);
+extern struct config_generic **get_modified_guc_options(int *num);
#endif /* GUC_TABLES_H */
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: explain plans with information about (modified) gucs
In-Reply-To: <631ae5c8-5581-6c5e-382c-2dcdb2e5c956@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox