Improve design and implementation of pg_file_settings view.

As first committed, this view reported on the file contents as they were
at the last SIGHUP event.  That's not as useful as reporting on the current
contents, and what's more, it didn't work right on Windows unless the
current session had serviced at least one SIGHUP.  Therefore, arrange to
re-read the files when pg_show_all_settings() is called.  This requires
only minor refactoring so that we can pass changeVal = false to
set_config_option() so that it won't actually apply any changes locally.

In addition, add error reporting so that errors that would prevent the
configuration files from being loaded, or would prevent individual settings
from being applied, are visible directly in the view.  This makes the view
usable for pre-testing whether edits made in the config files will have the
desired effect, before one actually issues a SIGHUP.

I also added an "applied" column so that it's easy to identify entries that
are superseded by later entries; this was the main use-case for the original
design, but it seemed unnecessarily hard to use for that.

Also fix a 9.4.1 regression that allowed multiple entries for a
PGC_POSTMASTER variable to cause bogus complaints in the postmaster log.
(The issue here was that commit bf007a27ac unintentionally reverted
3e3f65973a, which suppressed any duplicate entries within
ParseConfigFp.  However, since the original coding of the pg_file_settings
view depended on such suppression *not* happening, we couldn't have fixed
this issue now without first doing something with pg_file_settings.
Now we suppress duplicates by marking them "ignored" within
ProcessConfigFileInternal, which doesn't hide them in the view.)

Lesser changes include:

Drive the view directly off the ConfigVariable list, instead of making a
basically-equivalent second copy of the data.  There's no longer any need
to hang onto the data permanently, anyway.

Convert show_all_file_settings() to do its work in one call and return a
tuplestore; this avoids risks associated with assuming that the GUC state
will hold still over the course of query execution.  (I think there were
probably latent bugs here, though you might need something like a cursor
on the view to expose them.)

Arrange to run SIGHUP processing in a short-lived memory context, to
forestall process-lifespan memory leaks.  (There is one known leak in this
code, in ProcessConfigDirectory; it seems minor enough to not be worth
back-patching a specific fix for.)

Remove mistaken assignment to ConfigFileLineno that caused line counting
after an include_dir directive to be completely wrong.

Add missed failure check in AlterSystemSetConfigFile().  We don't really
expect ParseConfigFp() to fail, but that's not an excuse for not checking.

doc/src/sgml/catalogs.sgml

@@ -7626,7 +7626,7 @@
 
      <row>
       <entry><link linkend="view-pg-file-settings"><structname>pg_file_settings</structname></link></entry>
-      <entry>file location of parameter settings</entry>
+      <entry>summary of configuration file contents</entry>
      </row>
 
      <row>
@@ -8007,13 +8007,26 @@
   </indexterm>
 
   <para>
-   The view <structname>pg_file_settings</structname> provides the file
-   name, line number and value of all parameters which are set through
-   configuration files.
-   In contrast to <structname>pg_settings</structname>, a row is provided for
-   each occurrence of the parameter across all configuration files. This is helpful
-   for discovering why one value may have been used in preference to another
-   when the parameters were loaded.
+   The view <structname>pg_file_settings</structname> provides a summary of
+   the contents of the server's configuration file(s).  A row appears in
+   this view for each <quote>name = value</> entry appearing in the files,
+   with annotations indicating whether the value could be applied
+   successfully.  Additional row(s) may appear for problems not linked to
+   a <quote>name = value</> entry, such as syntax errors in the files.
+  </para>
+
+  <para>
+   This view is helpful for checking whether planned changes in the
+   configuration files will work, or for diagnosing a previous failure.
+   Note that this view reports on the <emphasis>current</> contents of the
+   files, not on what was last applied by the server.  (The
+   <link linkend="view-pg-settings"><structname>pg_settings</structname></link>
+   view is usually sufficient to determine that.)
+  </para>
+
+  <para>
+   The <structname>pg_file_settings</structname> view can be read only by
+   superusers.
   </para>
 
   <table>
@@ -8031,43 +8044,64 @@
     <row>
      <entry><structfield>sourcefile</structfield></entry>
      <entry><structfield>text</structfield></entry>
-     <entry>Path to and name of the configration file</entry>
+     <entry>Full pathname of the configuration file</entry>
     </row>
     <row>
      <entry><structfield>sourceline</structfield></entry>
      <entry><structfield>integer</structfield></entry>
      <entry>
-      Line number within the configuration file where the value was set
+      Line number within the configuration file where the entry appears
      </entry>
     </row>
     <row>
      <entry><structfield>seqno</structfield></entry>
      <entry><structfield>integer</structfield></entry>
-     <entry>Order in which the setting was loaded</entry>
+     <entry>Order in which the entries are processed (1..<replaceable>n</>)</entry>
     </row>
     <row>
      <entry><structfield>name</structfield></entry>
      <entry><structfield>text</structfield></entry>
-     <entry>Run-time configuration parameter name</entry>
+     <entry>Configuration parameter name</entry>
     </row>
     <row>
      <entry><structfield>setting</structfield></entry>
      <entry><structfield>text</structfield></entry>
-     <entry>value of the parameter</entry>
+     <entry>Value to be assigned to the parameter</entry>
+    </row>
+    <row>
+     <entry><structfield>applied</structfield></entry>
+     <entry><structfield>boolean</structfield></entry>
+     <entry>True if the value can be applied successfully</entry>
+    </row>
+    <row>
+     <entry><structfield>error</structfield></entry>
+     <entry><structfield>text</structfield></entry>
+     <entry>If not null, an error message indicating why this entry could
+      not be applied</entry>
     </row>
    </tbody>
   </tgroup>
- </table>
+  </table>
 
   <para>
-   See <xref linkend="config-setting"> for more information about the various
-   ways to change these parameters.
+   If the configuration file contains syntax errors or invalid parameter
+   names, the server will not attempt to apply any settings from it, and
+   therefore all the <structfield>applied</> fields will read as false.
+   In such a case there will be one or more rows with
+   non-null <structfield>error</structfield> fields indicating the
+   problem(s).  Otherwise, individual settings will be applied if possible.
+   If an individual setting cannot be applied (e.g., invalid value, or the
+   setting cannot be changed after server start) it will have an appropriate
+   message in the <structfield>error</structfield> field.  Another way that
+   an entry might have <structfield>applied</> = false is that it is
+   overridden by a later entry for the same parameter name; this case is not
+   considered an error so nothing appears in
+   the <structfield>error</structfield> field.
   </para>
 
   <para>
-   The <structname>pg_file_settings</structname> view cannot be modified
-   directly as it represents information, as read in at server start or
-   reload time, about all parameter settings across all configuration files.
+   See <xref linkend="config-setting"> for more information about the various
+   ways to change run-time parameters.
   </para>
 
 </sect1>

doc/src/sgml/config.sgml

@@ -175,6 +175,14 @@ shared_buffers = 128MB
      effect in the same way.  Settings in <filename>postgresql.auto.conf</>
      override those in <filename>postgresql.conf</>.
     </para>
+
+    <para>
+     The system view
+     <link linkend="view-pg-file-settings"><structname>pg_file_settings</structname></link>
+     can be helpful for pre-testing changes to the configuration file, or for
+     diagnosing problems if a <systemitem>SIGHUP</> signal did not have the
+     desired effects.
+    </para>
    </sect2>
 
    <sect2 id="config-setting-sql-command-interaction">

doc/src/sgml/release-9.5.sgml

@@ -6,8 +6,8 @@
 
   <note>
    <title>Release Date</title>
-   <para>AS OF 2015-06-01</para>
-   <simpara>2015-XX-XX</simpara>
+   <simpara>2015-??-??</simpara>
+   <simpara>Current as of 2015-06-01</simpara>
   </note>
 
   <sect2>
@@ -23,6 +23,7 @@
 
     <listitem>
      <para>
+      ... to be filled in ...
      </para>
     </listitem>
 
@@ -480,10 +481,10 @@
 
       <listitem>
        <para>
-        Add function and view <link
-        linkend="view-pg-file-settings"><function>pg_file_settings</></>
-        to show the source of <acronym>GUC</> values set in configuration
-        files (Sawada Masahiko)
+        Add system view <link
+        linkend="view-pg-file-settings"><structname>pg_file_settings</></>
+        to show the contents of the server's configuration files
+        (Sawada Masahiko)
        </para>
       </listitem>
 
@@ -503,7 +504,7 @@
        </para>
 
        <para>
-        This removes the setting from <filename>postgresql.auto.conf</>.
+        This command removes the setting from <filename>postgresql.auto.conf</>.
        </para>
       </listitem>
 

src/backend/utils/misc/guc-file.l

@@ -47,6 +47,12 @@ static sigjmp_buf *GUC_flex_fatal_jmp;
 
 static void FreeConfigVariable(ConfigVariable *item);
 
+static void record_config_file_error(const char *errmsg,
+						 const char *config_file,
+						 int lineno,
+						 ConfigVariable **head_p,
+						 ConfigVariable **tail_p);
+
 static int GUC_flex_fatal(const char *msg);
 static char *GUC_scanstr(const char *s);
 
@@ -107,20 +113,15 @@ STRING			\'([^'\\\n]|\\.|\'\')*\'
  * parameter indicates in what context the file is being read --- either
  * postmaster startup (including standalone-backend startup) or SIGHUP.
  * All options mentioned in the configuration file are set to new values.
- * If an error occurs, no values will be changed.
+ * If a hard error occurs, no values will be changed.  (There can also be
+ * errors that prevent just one value from being changed.)
  */
 void
 ProcessConfigFile(GucContext context)
 {
-	bool		error = false;
-	bool		apply = false;
 	int			elevel;
-	const char *ConfFileWithError;
-	ConfigVariable *item,
-			   *head,
-			   *tail;
-	int			i;
-	int			file_variables_count = 0;
+	MemoryContext config_cxt;
+	MemoryContext caller_cxt;
 
 	/*
 	 * Config files are processed on startup (by the postmaster only)
@@ -135,15 +136,58 @@ ProcessConfigFile(GucContext context)
 	 */
 	elevel = IsUnderPostmaster ? DEBUG2 : LOG;
 
+	/*
+	 * This function is usually called within a process-lifespan memory
+	 * context.  To ensure that any memory leaked during GUC processing does
+	 * not accumulate across repeated SIGHUP cycles, do the work in a private
+	 * context that we can free at exit.
+	 */
+	config_cxt = AllocSetContextCreate(CurrentMemoryContext,
+									   "config file processing",
+									   ALLOCSET_DEFAULT_MINSIZE,
+									   ALLOCSET_DEFAULT_MINSIZE,
+									   ALLOCSET_DEFAULT_MAXSIZE);
+	caller_cxt = MemoryContextSwitchTo(config_cxt);
+
+	/*
+	 * Read and apply the config file.  We don't need to examine the result.
+	 */
+	(void) ProcessConfigFileInternal(context, true, elevel);
+
+	/* Clean up */
+	MemoryContextSwitchTo(caller_cxt);
+	MemoryContextDelete(config_cxt);
+}
+
+/*
+ * This function handles both actual config file (re)loads and execution of
+ * show_all_file_settings() (i.e., the pg_file_settings view).  In the latter
+ * case we don't apply any of the settings, but we make all the usual validity
+ * checks, and we return the ConfigVariable list so that it can be printed out
+ * by show_all_file_settings().
+ */
+static ConfigVariable *
+ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
+{
+	bool		error = false;
+	bool		applying = false;
+	const char *ConfFileWithError;
+	ConfigVariable *item,
+			   *head,
+			   *tail;
+	int			i;
+
 	/* Parse the main config file into a list of option names and values */
 	ConfFileWithError = ConfigFileName;
 	head = tail = NULL;
 
-	if (!ParseConfigFile(ConfigFileName, NULL, true, 0, elevel, &head, &tail))
+	if (!ParseConfigFile(ConfigFileName, true,
+						 NULL, 0, 0, elevel,
+						 &head, &tail))
 	{
 		/* Syntax error(s) detected in the file, so bail out */
 		error = true;
-		goto cleanup_list;
+		goto bail_out;
 	}
 
 	/*
@@ -154,13 +198,14 @@ ProcessConfigFile(GucContext context)
 	 */
 	if (DataDir)
 	{
-		if (!ParseConfigFile(PG_AUTOCONF_FILENAME, NULL, false, 0, elevel,
+		if (!ParseConfigFile(PG_AUTOCONF_FILENAME, false,
+							 NULL, 0, 0, elevel,
 							 &head, &tail))
 		{
 			/* Syntax error(s) detected in the file, so bail out */
 			error = true;
 			ConfFileWithError = PG_AUTOCONF_FILENAME;
-			goto cleanup_list;
+			goto bail_out;
 		}
 	}
 	else
@@ -173,28 +218,22 @@ ProcessConfigFile(GucContext context)
 		 * will be read later. OTOH, since data_directory isn't allowed in the
 		 * PG_AUTOCONF_FILENAME file, it will never be overwritten later.
 		 */
-		ConfigVariable *prev = NULL;
+		ConfigVariable *newlist = NULL;
 
-		/* Prune all items except "data_directory" from the list */
-		for (item = head; item;)
+		/*
+		 * Prune all items except the last "data_directory" from the list.
+		 */
+		for (item = head; item; item = item->next)
 		{
-			ConfigVariable *ptr = item;
-
-			item = item->next;
-			if (strcmp(ptr->name, "data_directory") != 0)
-			{
-				if (prev == NULL)
-					head = ptr->next;
-				else
-					prev->next = ptr->next;
-				if (ptr->next == NULL)
-					tail = prev;
-				FreeConfigVariable(ptr);
-			}
-			else
-				prev = ptr;
+			if (!item->ignore &&
+				strcmp(item->name, "data_directory") == 0)
+				newlist = item;
 		}
 
+		if (newlist)
+			newlist->next = NULL;
+		head = tail = newlist;
+
 		/*
 		 * Quick exit if data_directory is not present in file.
 		 *
@@ -203,7 +242,7 @@ ProcessConfigFile(GucContext context)
 		 * the config file.
 		 */
 		if (head == NULL)
-			return;
+			goto bail_out;
 	}
 
 	/*
@@ -228,12 +267,17 @@ ProcessConfigFile(GucContext context)
 	 * same reason, we don't attempt to validate the options' values here.
 	 *
 	 * In addition, the GUC_IS_IN_FILE flag is set on each existing GUC
-	 * variable mentioned in the file.
+	 * variable mentioned in the file; and we detect duplicate entries in
+	 * the file and mark the earlier occurrences as ignorable.
 	 */
 	for (item = head; item; item = item->next)
 	{
 		struct config_generic *record;
 
+		/* Ignore anything already marked as ignorable */
+		if (item->ignore)
+			continue;
+
 		/*
 		 * Try to find the variable; but do not create a custom placeholder
 		 * if it's not there already.
@@ -242,7 +286,24 @@ ProcessConfigFile(GucContext context)
 
 		if (record)
 		{
-			/* Found, so mark it as present in file */
+			/* If it's already marked, then this is a duplicate entry */
+			if (record->status & GUC_IS_IN_FILE)
+			{
+				/*
+				 * Mark the earlier occurrence(s) as dead/ignorable.  We could
+				 * avoid the O(N^2) behavior here with some additional state,
+				 * but it seems unlikely to be worth the trouble.
+				 */
+				ConfigVariable *pitem;
+
+				for (pitem = head; pitem != item; pitem = pitem->next)
+				{
+					if (!pitem->ignore &&
+						strcmp(pitem->name, item->name) == 0)
+						pitem->ignore = true;
+				}
+			}
+			/* Now mark it as present in file */
 			record->status |= GUC_IS_IN_FILE;
 		}
 		else if (strchr(item->name, GUC_QUALIFIER_SEPARATOR) == NULL)
@@ -253,10 +314,10 @@ ProcessConfigFile(GucContext context)
 					 errmsg("unrecognized configuration parameter \"%s\" in file \"%s\" line %u",
 							item->name,
 							item->filename, item->sourceline)));
+			item->errmsg = pstrdup("unrecognized configuration parameter");
 			error = true;
 			ConfFileWithError = item->filename;
 		}
-		file_variables_count++;
 	}
 
 	/*
@@ -264,10 +325,10 @@ ProcessConfigFile(GucContext context)
 	 * any changes.
 	 */
 	if (error)
-		goto cleanup_list;
+		goto bail_out;
 
 	/* Otherwise, set flag that we're beginning to apply changes */
-	apply = true;
+	applying = true;
 
 	/*
 	 * Check for variables having been removed from the config file, and
@@ -289,10 +350,18 @@ ProcessConfigFile(GucContext context)
 					(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
 					 errmsg("parameter \"%s\" cannot be changed without restarting the server",
 							gconf->name)));
+			record_config_file_error(psprintf("parameter \"%s\" cannot be changed without restarting the server",
+											  gconf->name),
+									 NULL, 0,
+									 &head, &tail);
 			error = true;
 			continue;
 		}
 
+		/* No more to do if we're just doing show_all_file_settings() */
+		if (!applySettings)
+			continue;
+
 		/*
 		 * Reset any "file" sources to "default", else set_config_option
 		 * will not override those settings.
@@ -334,7 +403,7 @@ ProcessConfigFile(GucContext context)
 	 * potentially have PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR source.
 	 * However, there's no time to redesign it for 9.1.
 	 */
-	if (context == PGC_SIGHUP)
+	if (context == PGC_SIGHUP && applySettings)
 	{
 		InitializeGUCOptionsFromEnvironment();
 		pg_timezone_abbrev_initialize();
@@ -343,54 +412,6 @@ ProcessConfigFile(GucContext context)
 						PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT);
 	}
 
-	/*
-	 * Check if we have allocated the array yet.
-	 *
-	 * If not, allocate it based on the number of file variables we have seen.
-	 */
-	if (!guc_file_variables)
-	{
-		/* For the first call */
-		num_guc_file_variables = file_variables_count;
-		guc_file_variables = (ConfigFileVariable *) guc_malloc(FATAL,
-					num_guc_file_variables * sizeof(struct ConfigFileVariable));
-	}
-	else
-	{
-		int i;
-
-		/* Free all of the previously allocated entries */
-		for (i = 0; i < num_guc_file_variables; i++)
-		{
-			free(guc_file_variables[i].name);
-			free(guc_file_variables[i].value);
-			free(guc_file_variables[i].filename);
-		}
-
-		/* Update the global count and realloc based on the new size */
-		num_guc_file_variables = file_variables_count;
-		guc_file_variables = (ConfigFileVariable *) guc_realloc(FATAL,
-									guc_file_variables,
-					num_guc_file_variables * sizeof(struct ConfigFileVariable));
-	}
-
-	/*
-	 * Copy the settings which came from the files read into the
-	 * guc_file_variables array which backs the pg_show_file_settings()
-	 * function.
-	 */
-	for (item = head, i = 0; item && i < num_guc_file_variables;
-		 item = item->next, i++)
-	{
-		guc_file_variables[i].name = guc_strdup(FATAL, item->name);
-		guc_file_variables[i].value = guc_strdup(FATAL, item->value);
-		guc_file_variables[i].filename = guc_strdup(FATAL, item->filename);
-		guc_file_variables[i].sourceline = item->sourceline;
-	}
-
-	/* We had better have made it through the loop above to a clean ending. */
-	Assert(!item && i == num_guc_file_variables);
-
 	/*
 	 * Now apply the values from the config file.
 	 */
@@ -399,8 +420,12 @@ ProcessConfigFile(GucContext context)
 		char   *pre_value = NULL;
 		int		scres;
 
+		/* Ignore anything marked as ignorable */
+		if (item->ignore)
+			continue;
+
 		/* In SIGHUP cases in the postmaster, we want to report changes */
-		if (context == PGC_SIGHUP && !IsUnderPostmaster)
+		if (context == PGC_SIGHUP && applySettings && !IsUnderPostmaster)
 		{
 			const char *preval = GetConfigOption(item->name, true, false);
 
@@ -413,7 +438,7 @@ ProcessConfigFile(GucContext context)
 
 		scres = set_config_option(item->name, item->value,
 								  context, PGC_S_FILE,
-								  GUC_ACTION_SET, true, 0, false);
+								  GUC_ACTION_SET, applySettings, 0, false);
 		if (scres > 0)
 		{
 			/* variable was updated, so log the change if appropriate */
@@ -428,13 +453,19 @@ ProcessConfigFile(GucContext context)
 							(errmsg("parameter \"%s\" changed to \"%s\"",
 									item->name, item->value)));
 			}
+			item->applied = true;
 		}
 		else if (scres == 0)
 		{
 			error = true;
+			item->errmsg = pstrdup("setting could not be applied");
 			ConfFileWithError = item->filename;
 		}
-		/* else no error but variable's active value was not changed */
+		else
+		{
+			/* no error, but variable's active value was not changed */
+			item->applied = true;
+		}
 
 		/*
 		 * We should update source location unless there was an error, since
@@ -442,7 +473,7 @@ ProcessConfigFile(GucContext context)
 		 * (In the postmaster, there won't be a difference, but it does matter
 		 * in backends.)
 		 */
-		if (scres != 0)
+		if (scres != 0 && applySettings)
 			set_config_sourcefile(item->name, item->filename,
 								  item->sourceline);
 
@@ -451,10 +482,11 @@ ProcessConfigFile(GucContext context)
 	}
 
 	/* Remember when we last successfully loaded the config file. */
-	PgReloadTime = GetCurrentTimestamp();
+	if (applySettings)
+		PgReloadTime = GetCurrentTimestamp();
 
- cleanup_list:
-	if (error)
+ bail_out:
+	if (error && applySettings)
 	{
 		/* During postmaster startup, any error is fatal */
 		if (context == PGC_POSTMASTER)
@@ -462,7 +494,7 @@ ProcessConfigFile(GucContext context)
 					(errcode(ERRCODE_CONFIG_FILE_ERROR),
 					 errmsg("configuration file \"%s\" contains errors",
 							ConfFileWithError)));
-		else if (apply)
+		else if (applying)
 			ereport(elevel,
 					(errcode(ERRCODE_CONFIG_FILE_ERROR),
 					 errmsg("configuration file \"%s\" contains errors; unaffected changes were applied",
@@ -474,12 +506,8 @@ ProcessConfigFile(GucContext context)
 							ConfFileWithError)));
 	}
 
-	/*
-	 * Calling FreeConfigVariables() any earlier than this can cause problems,
-	 * because ConfFileWithError could be pointing to a string that will be
-	 * freed here.
-	 */
-	FreeConfigVariables(head);
+	/* Successful or otherwise, return the collected data list */
+	return head;
 }
 
 /*
@@ -520,12 +548,16 @@ AbsoluteConfigLocation(const char *location, const char *calling_file)
  * If "strict" is true, treat failure to open the config file as an error,
  * otherwise just skip the file.
  *
+ * calling_file/calling_lineno identify the source of the request.
+ * Pass NULL/0 if not recursing from an inclusion request.
+ *
  * See ParseConfigFp for further details.  This one merely adds opening the
  * config file rather than working from a caller-supplied file descriptor,
  * and absolute-ifying the path name if necessary.
  */
 bool
-ParseConfigFile(const char *config_file, const char *calling_file, bool strict,
+ParseConfigFile(const char *config_file, bool strict,
+				const char *calling_file, int calling_lineno,
 				int depth, int elevel,
 				ConfigVariable **head_p,
 				ConfigVariable **tail_p)
@@ -545,6 +577,9 @@ ParseConfigFile(const char *config_file, const char *calling_file, bool strict,
 				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 				 errmsg("could not open configuration file \"%s\": maximum nesting depth exceeded",
 						config_file)));
+		record_config_file_error("nesting depth exceeded",
+								 calling_file, calling_lineno,
+								 head_p, tail_p);
 		return false;
 	}
 
@@ -558,6 +593,10 @@ ParseConfigFile(const char *config_file, const char *calling_file, bool strict,
 					(errcode_for_file_access(),
 					 errmsg("could not open configuration file \"%s\": %m",
 							abs_path)));
+			record_config_file_error(psprintf("could not open file \"%s\"",
+											  abs_path),
+									 calling_file, calling_lineno,
+									 head_p, tail_p);
 			OK = false;
 		}
 		else
@@ -579,6 +618,35 @@ cleanup:
 	return OK;
 }
 
+/*
+ * Capture an error message in the ConfigVariable list returned by
+ * config file parsing.
+ */
+static void
+record_config_file_error(const char *errmsg,
+						 const char *config_file,
+						 int lineno,
+						 ConfigVariable **head_p,
+						 ConfigVariable **tail_p)
+{
+	ConfigVariable *item;
+
+	item = palloc(sizeof *item);
+	item->name = NULL;
+	item->value = NULL;
+	item->errmsg = pstrdup(errmsg);
+	item->filename = config_file ? pstrdup(config_file) : NULL;
+	item->sourceline = lineno;
+	item->ignore = true;
+	item->applied = false;
+	item->next = NULL;
+	if (*head_p == NULL)
+		*head_p = item;
+	else
+		(*tail_p)->next = item;
+	*tail_p = item;
+}
+
 /*
  * Flex fatal errors bring us here.  Stash the error message and jump back to
  * ParseConfigFp().  Assume all msg arguments point to string constants; this
@@ -607,9 +675,10 @@ GUC_flex_fatal(const char *msg)
  * Input/Output parameters:
  *	head_p, tail_p: head and tail of linked list of name/value pairs
  *
- * *head_p and *tail_p must either be initialized to NULL or valid pointers
- * to a ConfigVariable list before calling the outer recursion level.  Any
- * name-value pairs read from the input file(s) will be added to the list.
+ * *head_p and *tail_p must be initialized, either to NULL or valid pointers
+ * to a ConfigVariable list, before calling the outer recursion level.  Any
+ * name-value pairs read from the input file(s) will be appended to the list.
+ * Error reports will also be appended to the list, if elevel < ERROR.
  *
  * Returns TRUE if successful, FALSE if an error occurred.  The error has
  * already been ereport'd, it is only necessary for the caller to clean up
@@ -617,6 +686,12 @@ GUC_flex_fatal(const char *msg)
  *
  * Note: if elevel >= ERROR then an error will not return control to the
  * caller, so there is no need to check the return value in that case.
+ *
+ * Note: this function is used to parse not only postgresql.conf, but
+ * various other configuration files that use the same "name = value"
+ * syntax.  Hence, do not do anything here or in the subsidiary routines
+ * ParseConfigFile/ParseConfigDirectory that assumes we are processing
+ * GUCs specifically.
  */
 bool
 ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
@@ -641,7 +716,9 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 		 */
 		elog(elevel, "%s at file \"%s\" line %u",
 			 GUC_flex_fatal_errmsg, config_file, ConfigFileLineno);
-
+		record_config_file_error(GUC_flex_fatal_errmsg,
+								 config_file, ConfigFileLineno,
+								 head_p, tail_p);
 		OK = false;
 		goto cleanup;
 	}
@@ -704,12 +781,12 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 			 * An include_dir directive isn't a variable and should be
 			 * processed immediately.
 			 */
-			if (!ParseConfigDirectory(opt_value, config_file,
-								 depth + 1, elevel,
-								 head_p, tail_p))
+			if (!ParseConfigDirectory(opt_value,
+									  config_file, ConfigFileLineno - 1,
+									  depth + 1, elevel,
+									  head_p, tail_p))
 				OK = false;
 			yy_switch_to_buffer(lex_buffer);
-			ConfigFileLineno = save_ConfigFileLineno;
 			pfree(opt_name);
 			pfree(opt_value);
 		}
@@ -719,7 +796,8 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 			 * An include_if_exists directive isn't a variable and should be
 			 * processed immediately.
 			 */
-			if (!ParseConfigFile(opt_value, config_file, false,
+			if (!ParseConfigFile(opt_value, false,
+								 config_file, ConfigFileLineno - 1,
 								 depth + 1, elevel,
 								 head_p, tail_p))
 				OK = false;
@@ -733,7 +811,8 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 			 * An include directive isn't a variable and should be processed
 			 * immediately.
 			 */
-			if (!ParseConfigFile(opt_value, config_file, true,
+			if (!ParseConfigFile(opt_value, true,
+								 config_file, ConfigFileLineno - 1,
 								 depth + 1, elevel,
 								 head_p, tail_p))
 				OK = false;
@@ -747,8 +826,11 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 			item = palloc(sizeof *item);
 			item->name = opt_name;
 			item->value = opt_value;
+			item->errmsg = NULL;
 			item->filename = pstrdup(config_file);
 			item->sourceline = ConfigFileLineno-1;
+			item->ignore = false;
+			item->applied = false;
 			item->next = NULL;
 			if (*head_p == NULL)
 				*head_p = item;
@@ -771,15 +853,25 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 
 		/* report the error */
 		if (token == GUC_EOL || token == 0)
+		{
 			ereport(elevel,
 					(errcode(ERRCODE_SYNTAX_ERROR),
 					 errmsg("syntax error in file \"%s\" line %u, near end of line",
 							config_file, ConfigFileLineno - 1)));
+			record_config_file_error("syntax error",
+									 config_file, ConfigFileLineno - 1,
+									 head_p, tail_p);
+		}
 		else
+		{
 			ereport(elevel,
 					(errcode(ERRCODE_SYNTAX_ERROR),
 					 errmsg("syntax error in file \"%s\" line %u, near token \"%s\"",
 							config_file, ConfigFileLineno, yytext)));
+			record_config_file_error("syntax error",
+									 config_file, ConfigFileLineno,
+									 head_p, tail_p);
+		}
 		OK = false;
 		errorcount++;
 
@@ -817,10 +909,17 @@ cleanup:
 
 /*
  * Read and parse all config files in a subdirectory in alphabetical order
+ *
+ * includedir is the absolute or relative path to the subdirectory to scan.
+ *
+ * calling_file/calling_lineno identify the source of the request.
+ * Pass NULL/0 if not recursing from an inclusion request.
+ *
+ * See ParseConfigFp for further details.
  */
 bool
 ParseConfigDirectory(const char *includedir,
-					 const char *calling_file,
+					 const char *calling_file, int calling_lineno,
 					 int depth, int elevel,
 					 ConfigVariable **head_p,
 					 ConfigVariable **tail_p)
@@ -828,9 +927,9 @@ ParseConfigDirectory(const char *includedir,
 	char	   *directory;
 	DIR		   *d;
 	struct dirent *de;
-	char	  **filenames = NULL;
-	int			num_filenames = 0;
-	int			size_filenames = 0;
+	char	  **filenames;
+	int			num_filenames;
+	int			size_filenames;
 	bool		status;
 
 	directory = AbsoluteConfigLocation(includedir, calling_file);
@@ -841,6 +940,10 @@ ParseConfigDirectory(const char *includedir,
 				(errcode_for_file_access(),
 				 errmsg("could not open configuration directory \"%s\": %m",
 						directory)));
+		record_config_file_error(psprintf("could not open directory \"%s\"",
+										  directory),
+								 calling_file, calling_lineno,
+								 head_p, tail_p);
 		status = false;
 		goto cleanup;
 	}
@@ -849,6 +952,10 @@ ParseConfigDirectory(const char *includedir,
 	 * Read the directory and put the filenames in an array, so we can sort
 	 * them prior to processing the contents.
 	 */
+	size_filenames = 32;
+	filenames = (char **) palloc(size_filenames * sizeof(char *));
+	num_filenames = 0;
+
 	while ((de = ReadDir(d, directory)) != NULL)
 	{
 		struct stat st;
@@ -872,15 +979,12 @@ ParseConfigDirectory(const char *includedir,
 		{
 			if (!S_ISDIR(st.st_mode))
 			{
-				/* Add file to list, increasing its size in blocks of 32 */
-				if (num_filenames == size_filenames)
+				/* Add file to array, increasing its size in blocks of 32 */
+				if (num_filenames >= size_filenames)
 				{
 					size_filenames += 32;
-					if (num_filenames == 0)
-						/* Must initialize, repalloc won't take NULL input */
-						filenames = palloc(size_filenames * sizeof(char *));
-					else
-						filenames = repalloc(filenames, size_filenames * sizeof(char *));
+					filenames = (char **) repalloc(filenames,
+												   size_filenames * sizeof(char *));
 				}
 				filenames[num_filenames] = pstrdup(filename);
 				num_filenames++;
@@ -897,6 +1001,10 @@ ParseConfigDirectory(const char *includedir,
 					(errcode_for_file_access(),
 					 errmsg("could not stat file \"%s\": %m",
 							filename)));
+			record_config_file_error(psprintf("could not stat file \"%s\"",
+											  filename),
+									 calling_file, calling_lineno,
+									 head_p, tail_p);
 			status = false;
 			goto cleanup;
 		}
@@ -908,8 +1016,10 @@ ParseConfigDirectory(const char *includedir,
 		qsort(filenames, num_filenames, sizeof(char *), pg_qsort_strcmp);
 		for (i = 0; i < num_filenames; i++)
 		{
-			if (!ParseConfigFile(filenames[i], NULL, true,
-								 depth, elevel, head_p, tail_p))
+			if (!ParseConfigFile(filenames[i], true,
+								 calling_file, calling_lineno,
+								 depth, elevel,
+								 head_p, tail_p))
 			{
 				status = false;
 				goto cleanup;
@@ -949,9 +1059,14 @@ FreeConfigVariables(ConfigVariable *list)
 static void
 FreeConfigVariable(ConfigVariable *item)
 {
-	pfree(item->name);
-	pfree(item->value);
-	pfree(item->filename);
+	if (item->name)
+		pfree(item->name);
+	if (item->value)
+		pfree(item->value);
+	if (item->errmsg)
+		pfree(item->errmsg);
+	if (item->filename)
+		pfree(item->filename);
 	pfree(item);
 }
 

src/backend/utils/misc/guc.c

@@ -182,6 +182,10 @@ static bool check_cluster_name(char **newval, void **extra, GucSource source);
 static const char *show_unix_socket_permissions(void);
 static const char *show_log_file_mode(void);
 
+/* Private functions in guc-file.l that need to be called from guc.c */
+static ConfigVariable *ProcessConfigFileInternal(GucContext context,
+						  bool applySettings, int elevel);
+
 
 /*
  * Options for enum values defined in this module.
@@ -3678,22 +3682,6 @@ static struct config_generic **guc_variables;
 /* Current number of variables contained in the vector */
 static int	num_guc_variables;
 
-/*
- * Lookup of variables for pg_file_settings view.
- * guc_file_variables is an array of length num_guc_file_variables.
- */
-typedef struct ConfigFileVariable
-{
-	char	   *name;
-	char	   *value;
-	char	   *filename;
-	int			sourceline;
-} ConfigFileVariable;
-static struct ConfigFileVariable *guc_file_variables;
-
-/* Number of file variables */
-static int	num_guc_file_variables;
-
 /* Vector capacity */
 static int	size_guc_variables;
 
@@ -6781,8 +6769,11 @@ replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p,
 	item = palloc(sizeof *item);
 	item->name = pstrdup(name);
 	item->value = pstrdup(value);
+	item->errmsg = NULL;
 	item->filename = pstrdup("");		/* new item has no location */
 	item->sourceline = 0;
+	item->ignore = false;
+	item->applied = false;
 	item->next = NULL;
 
 	if (*head_p == NULL)
@@ -6931,7 +6922,10 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 								AutoConfFileName)));
 
 			/* parse it */
-			ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail);
+			if (!ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail))
+				ereport(ERROR,
+						(errmsg("could not parse contents of file \"%s\"",
+								AutoConfFileName)));
 
 			FreeFile(infile);
 		}
@@ -8181,10 +8175,12 @@ show_all_settings(PG_FUNCTION_ARGS)
 /*
  * show_all_file_settings
  *
- * returns a table of all parameter settings in all configuration files
- * which includes the config file path/name, the line number, a sequence number
- * indicating when we loaded it, the parameter name, and the value it is
- * set to.
+ * Returns a table of all parameter settings in all configuration files
+ * which includes the config file pathname, the line number, a sequence number
+ * indicating the order in which the settings were encountered, the parameter
+ * name and value, a bool showing if the value could be applied, and possibly
+ * an associated error message.  (For problems such as syntax errors, the
+ * parameter name/value might be NULL.)
  *
  * Note: no filtering is done here, instead we depend on the GRANT system
  * to prevent unprivileged users from accessing this function or the view
@@ -8193,92 +8189,111 @@ show_all_settings(PG_FUNCTION_ARGS)
 Datum
 show_all_file_settings(PG_FUNCTION_ARGS)
 {
-#define NUM_PG_FILE_SETTINGS_ATTS 5
-	FuncCallContext *funcctx;
+#define NUM_PG_FILE_SETTINGS_ATTS 7
+	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	TupleDesc	tupdesc;
-	int			call_cntr;
-	int			max_calls;
-	AttInMetadata *attinmeta;
+	Tuplestorestate *tupstore;
+	ConfigVariable *conf;
+	int			seqno;
+	MemoryContext per_query_ctx;
 	MemoryContext oldcontext;
 
-	if (SRF_IS_FIRSTCALL())
-	{
-		funcctx = SRF_FIRSTCALL_INIT();
-
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+	/* Check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsinfo->allowedModes & SFRM_Materialize))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("materialize mode required, but it is not " \
+						"allowed in this context")));
 
-		/*
-		 * need a tuple descriptor representing NUM_PG_FILE_SETTINGS_ATTS
-		 * columns of the appropriate types
-		 */
+	/* Scan the config files using current context as workspace */
+	conf = ProcessConfigFileInternal(PGC_SIGHUP, false, DEBUG3);
 
-		tupdesc = CreateTemplateTupleDesc(NUM_PG_FILE_SETTINGS_ATTS, false);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 1, "sourcefile",
-						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 2, "sourceline",
-						   INT4OID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 3, "seqno",
-						   INT4OID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 4, "name",
-						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 5, "setting",
-						   TEXTOID, -1, 0);
+	/* Switch into long-lived context to construct returned data structures */
+	per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
+	oldcontext = MemoryContextSwitchTo(per_query_ctx);
 
-		attinmeta = TupleDescGetAttInMetadata(tupdesc);
-		funcctx->attinmeta = attinmeta;
-		funcctx->max_calls = num_guc_file_variables;
-		MemoryContextSwitchTo(oldcontext);
-	}
+	/* Build a tuple descriptor for our result type */
+	tupdesc = CreateTemplateTupleDesc(NUM_PG_FILE_SETTINGS_ATTS, false);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 1, "sourcefile",
+					   TEXTOID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 2, "sourceline",
+					   INT4OID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 3, "seqno",
+					   INT4OID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 4, "name",
+					   TEXTOID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 5, "setting",
+					   TEXTOID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 6, "applied",
+					   BOOLOID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 7, "error",
+					   TEXTOID, -1, 0);
 
-	funcctx = SRF_PERCALL_SETUP();
+	/* Build a tuplestore to return our results in */
+	tupstore = tuplestore_begin_heap(true, false, work_mem);
+	rsinfo->returnMode = SFRM_Materialize;
+	rsinfo->setResult = tupstore;
+	rsinfo->setDesc = tupdesc;
 
-	call_cntr = funcctx->call_cntr;
-	max_calls = funcctx->max_calls;
-	attinmeta = funcctx->attinmeta;
+	/* The rest can be done in short-lived context */
+	MemoryContextSwitchTo(oldcontext);
 
-	if (call_cntr < max_calls)
+	/* Process the results and create a tuplestore */
+	for (seqno = 1; conf != NULL; conf = conf->next, seqno++)
 	{
-		char	   *values[NUM_PG_FILE_SETTINGS_ATTS];
-		HeapTuple	tuple;
-		Datum		result;
-		ConfigFileVariable conf;
-		char		buffer[12]; /* must be at least 12, per pg_ltoa */
+		Datum		values[NUM_PG_FILE_SETTINGS_ATTS];
+		bool		nulls[NUM_PG_FILE_SETTINGS_ATTS];
 
-		/* Check to avoid going past end of array */
-		if (call_cntr > num_guc_file_variables)
-			SRF_RETURN_DONE(funcctx);
-
-		conf = guc_file_variables[call_cntr];
+		memset(values, 0, sizeof(values));
+		memset(nulls, 0, sizeof(nulls));
 
 		/* sourcefile */
-		values[0] = conf.filename;
+		if (conf->filename)
+			values[0] = PointerGetDatum(cstring_to_text(conf->filename));
+		else
+			nulls[0] = true;
 
-		/* sourceline */
-		pg_ltoa(conf.sourceline, buffer);
-		values[1] = pstrdup(buffer);
+		/* sourceline (not meaningful if no sourcefile) */
+		if (conf->filename)
+			values[1] = Int32GetDatum(conf->sourceline);
+		else
+			nulls[1] = true;
 
 		/* seqno */
-		pg_ltoa(call_cntr + 1, buffer);
-		values[2] = pstrdup(buffer);
+		values[2] = Int32GetDatum(seqno);
 
 		/* name */
-		values[3] = conf.name;
+		if (conf->name)
+			values[3] = PointerGetDatum(cstring_to_text(conf->name));
+		else
+			nulls[3] = true;
 
 		/* setting */
-		values[4] = conf.value;
+		if (conf->value)
+			values[4] = PointerGetDatum(cstring_to_text(conf->value));
+		else
+			nulls[4] = true;
 
-		/* build a tuple */
-		tuple = BuildTupleFromCStrings(attinmeta, values);
+		/* applied */
+		values[5] = BoolGetDatum(conf->applied);
 
-		/* make the tuple into a datum */
-		result = HeapTupleGetDatum(tuple);
+		/* error */
+		if (conf->errmsg)
+			values[6] = PointerGetDatum(cstring_to_text(conf->errmsg));
+		else
+			nulls[6] = true;
 
-		SRF_RETURN_NEXT(funcctx, result);
-	}
-	else
-	{
-		SRF_RETURN_DONE(funcctx);
+		/* shove row into tuplestore */
+		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
+
+	tuplestore_donestoring(tupstore);
+
+	return (Datum) 0;
 }
 
 static char *

src/include/catalog/catversion.h

@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	201506281
+#define CATALOG_VERSION_NO	201506282
 
 #endif

src/include/catalog/pg_proc.h

@@ -3071,7 +3071,7 @@ DATA(insert OID = 2078 (  set_config		PGNSP PGUID 12 1 0 0 0 f f f f f f v 3 0 2
 DESCR("SET X as a function");
 DATA(insert OID = 2084 (  pg_show_all_settings	PGNSP PGUID 12 1 1000 0 0 f f f f t t s 0 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,1009,25,25,25,23,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,boot_val,reset_val,sourcefile,sourceline,pending_restart}" _null_ _null_ show_all_settings _null_ _null_ _null_ ));
 DESCR("SHOW ALL as a function");
-DATA(insert OID = 3329 (  pg_show_all_file_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t s 0 0 2249 "" "{25,23,23,25,25}" "{o,o,o,o,o}" "{sourcefile,sourceline,seqno,name,setting}" _null_ _null_ show_all_file_settings _null_ _null_ _null_ ));
+DATA(insert OID = 3329 (  pg_show_all_file_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t v 0 0 2249 "" "{25,23,23,25,25,16,25}" "{o,o,o,o,o,o,o}" "{sourcefile,sourceline,seqno,name,setting,applied,error}" _null_ _null_ show_all_file_settings _null_ _null_ _null_ ));
 DESCR("show config file settings");
 DATA(insert OID = 1371 (  pg_lock_status   PGNSP PGUID 12 1 1000 0 0 f f f f t t v 0 0 2249 "" "{25,26,26,23,21,25,28,26,26,21,25,23,25,16,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted,fastpath}" _null_ _null_ pg_lock_status _null_ _null_ _null_ ));
 DESCR("view system lock information");

src/include/utils/guc.h

@@ -121,26 +121,36 @@ typedef enum
 } GucSource;
 
 /*
- * Parsing the configuration file will return a list of name-value pairs
- * with source location info.
+ * Parsing the configuration file(s) will return a list of name-value pairs
+ * with source location info.  We also abuse this data structure to carry
+ * error reports about the config files.  An entry reporting an error will
+ * have errmsg != NULL, and might have NULLs for name, value, and/or filename.
+ *
+ * If "ignore" is true, don't attempt to apply the item (it might be an error
+ * report, or an item we determined to be duplicate).  "applied" is set true
+ * if we successfully applied, or could have applied, the setting.
  */
 typedef struct ConfigVariable
 {
 	char	   *name;
 	char	   *value;
+	char	   *errmsg;
 	char	   *filename;
 	int			sourceline;
+	bool		ignore;
+	bool		applied;
 	struct ConfigVariable *next;
 } ConfigVariable;
 
-extern bool ParseConfigFile(const char *config_file, const char *calling_file,
-				bool strict, int depth, int elevel,
+extern bool ParseConfigFile(const char *config_file, bool strict,
+				const char *calling_file, int calling_lineno,
+				int depth, int elevel,
 				ConfigVariable **head_p, ConfigVariable **tail_p);
 extern bool ParseConfigFp(FILE *fp, const char *config_file,
 			  int depth, int elevel,
 			  ConfigVariable **head_p, ConfigVariable **tail_p);
 extern bool ParseConfigDirectory(const char *includedir,
-					 const char *calling_file,
+					 const char *calling_file, int calling_lineno,
 					 int depth, int elevel,
 					 ConfigVariable **head_p,
 					 ConfigVariable **tail_p);

src/test/regress/expected/rules.out

@@ -1316,8 +1316,10 @@ pg_file_settings| SELECT a.sourcefile,
     a.sourceline,
     a.seqno,
     a.name,
-    a.setting
-   FROM pg_show_all_file_settings() a(sourcefile, sourceline, seqno, name, setting);
+    a.setting,
+    a.applied,
+    a.error
+   FROM pg_show_all_file_settings() a(sourcefile, sourceline, seqno, name, setting, applied, error);
 pg_group| SELECT pg_authid.rolname AS groname,
     pg_authid.oid AS grosysid,
     ARRAY( SELECT pg_auth_members.member