On 12/14/18 4:32 PM, Tomas Vondra wrote:
>
>
> On 12/14/18 4:21 PM, Tom Lane wrote:
>> Tomas Vondra <[email protected]> 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
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 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.sgml
index 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.c
index 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.c
index 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.h
index 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.h
index 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 */
