Hi,

every now and then I have to investigate an execution plan that is
strange in some way and I can't reproduce the same behavior. Usually
it's simply due to data distribution changing since the problem was
observed (say, after a nightly batch load/update).

In many cases it however may be due to some local GUC tweaks, usually
addressing some query specific issues (say, disabling nested loops or
lowering join_collapse_limit). I've repeatedly ran into cases where the
GUC was not properly reset to the "regular" value, and it's rather
difficult to identify this is what's happening. Or cases with different
per-user settings and connection pooling (SET SESSION AUTHORIZATION /
ROLE etc.).

So 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):

test=# explain (gucs) select * from t;

                                 QUERY PLAN
  --------------------------------------------------------------------
   Seq Scan on t  (cost=0.00..35.50 rows=2550 width=4)
   GUCs: application_name = 'x', client_encoding = 'UTF8',
         cpu_tuple_cost = '0.01'
   (2 rows)

Of course, this directly applies to auto_explain too, which gets a new
option log_gucs.

The patch is quite trivial, but there are about three open questions:

1) names of the options

I'm not particularly happy with calling the option "gucs" - it's an
acronym and many users have little idea what GUC stands for. So I think
a better name would be desirable, but I'm not sure what would that be.
Options? Parameters?

2) format of output

At this point the names/values are simply formatted into a one-line
string. That's not particularly readable, and it's not very useful for
the YAML/JSON formats I guess. So adding each modified GUC as an extra
text property would be better.

3) identifying modified (and interesting) GUCs

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.

For now I've used

  /* return only options that were modified (not as in config file) */
  if ((conf->source <= PGC_S_ARGV) || (conf->source == PGC_S_OVERRIDE))
    continue;

which generally does the right thing, although it also includes stuff
like application_name or client_encoding. But perhaps it'd be better to
whitelist the GUCs in some way, because some of the user-defined GUCs
may be sensitive and should not be included in plans.

Opinions?


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..b8cab69f71 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;
@@ -547,6 +550,37 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
 	/* Create textual dump of plan tree */
 	ExplainPrintPlan(es, queryDesc);
 
+	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);
+	}
+
 	if (es->summary && planduration)
 	{
 		double		plantime = INSTR_TIME_GET_DOUBLE(*planduration);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6fe1939881..fd4473d9d0 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8556,6 +8556,37 @@ 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++)
+	{
+		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;
+
+		/* return only options that were modified (not as in config file) */
+		if ((conf->source <= PGC_S_ARGV) || (conf->source == PGC_S_OVERRIDE))
+			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 */

Attachment: pEpkey.asc
Description: application/pgp-keys

Reply via email to