Re: Semantics of pg_file_settings view - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: Semantics of pg_file_settings view |
| Date | |
| Msg-id | 21862.1435420043@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: Semantics of pg_file_settings view (Robert Haas <robertmhaas@gmail.com>) |
| Responses |
Re: Semantics of pg_file_settings view
|
| List | pgsql-hackers |
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jun 26, 2015 at 4:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Combining this with my idea about preserving the ConfigVariable list,
>> I'm thinking that it would be a good idea for ProcessConfigFile() to
>> run in a context created for the purpose of processing the config files,
>> rather than blindly using the caller's context, which is likely to be
>> a process-lifespan context and thus not a good place to leak in.
>> We could keep this context around until the next SIGHUP event, so that
>> the ConfigVariable list remains available, and then destroy it and
>> replace it with the next ProcessConfigFile's instance of the context.
>> In this way, any leakage in the processing code could not accumulate
>> over multiple SIGHUPs, and so it would be certain to remain fairly
>> negligible.
> That seems like a nice idea.
Attached is a WIP version of this idea. It lacks docs, and there is one
further API change I'd like to discuss, but what is there so far is:
1. It implements the above idea of doing SIGHUP work in a dedicated
context that gets flushed at the next SIGHUP, and using the
ConfigVariables list directly as the source data for the pg_file_settings
view.
2. It adds an "error message" field to struct ConfigVariable, and a
corresponding column to the pg_file_settings view, allowing problems
to be reported. Some examples of what it can do:
Normal case with an initdb-generated postgresql.conf:
regression=# select * from pg_file_settings;
sourcefile | sourceline | seqno | name | setting
|error
-------------------------------------------------+------------+-------+----------------------------+--------------------+-------
/home/postgres/testversion/data/postgresql.conf | 64 | 1 | max_connections | 100
|
/home/postgres/testversion/data/postgresql.conf | 116 | 2 | shared_buffers | 128MB
|
/home/postgres/testversion/data/postgresql.conf | 131 | 3 | dynamic_shared_memory_type | posix
|
/home/postgres/testversion/data/postgresql.conf | 446 | 4 | log_timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 533 | 5 | datestyle | iso, mdy
|
/home/postgres/testversion/data/postgresql.conf | 535 | 6 | timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 548 | 7 | lc_messages | C
|
/home/postgres/testversion/data/postgresql.conf | 550 | 8 | lc_monetary | C
|
/home/postgres/testversion/data/postgresql.conf | 551 | 9 | lc_numeric | C
|
/home/postgres/testversion/data/postgresql.conf | 552 | 10 | lc_time | C
|
/home/postgres/testversion/data/postgresql.conf | 555 | 11 | default_text_search_config | pg_catalog.english
|
(11 rows)
postgresql.conf is not there/not readable:
regression=# select * from pg_file_settings;
sourcefile | sourceline | seqno | name | setting | error
------------+------------+-------+------+---------+-----------------------------------------------------------------------
| 0 | 1 | | | could not open file
"/home/postgres/testversion/data/postgresql.conf"
(1 row)
Bad include_dir entry:
sourcefile | sourceline | seqno | name | setting
| error
-------------------------------------------------+------------+-------+----------------------------+--------------------+--------------------------------------------------------------------
/home/postgres/testversion/data/postgresql.conf | 64 | 1 | max_connections | 100
|
/home/postgres/testversion/data/postgresql.conf | 116 | 2 | shared_buffers | 128MB
|
/home/postgres/testversion/data/postgresql.conf | 131 | 3 | dynamic_shared_memory_type | posix
|
/home/postgres/testversion/data/postgresql.conf | 446 | 4 | log_timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 533 | 5 | datestyle | iso, mdy
|
/home/postgres/testversion/data/postgresql.conf | 535 | 6 | timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 548 | 7 | lc_messages | C
|
/home/postgres/testversion/data/postgresql.conf | 550 | 8 | lc_monetary | C
|
/home/postgres/testversion/data/postgresql.conf | 551 | 9 | lc_numeric | C
|
/home/postgres/testversion/data/postgresql.conf | 552 | 10 | lc_time | C
|
/home/postgres/testversion/data/postgresql.conf | 555 | 11 | default_text_search_config | pg_catalog.english
|
/home/postgres/testversion/data/postgresql.conf | 626 | 12 | |
|could not open directory "/home/postgres/testversion/data/conf.xx"
(12 rows)
Bad value for a known GUC variable:
sourcefile | sourceline | seqno | name | setting
| error
-------------------------------------------------+------------+-------+----------------------------+--------------------+------------------------------
/home/postgres/testversion/data/postgresql.conf | 64 | 1 | max_connections | 100
|
/home/postgres/testversion/data/postgresql.conf | 116 | 2 | shared_buffers | 128MB
|
/home/postgres/testversion/data/postgresql.conf | 131 | 3 | dynamic_shared_memory_type | posix
|
/home/postgres/testversion/data/postgresql.conf | 446 | 4 | log_timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 533 | 5 | datestyle | iso, mdy
|
/home/postgres/testversion/data/postgresql.conf | 535 | 6 | timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 548 | 7 | lc_messages | C
|
/home/postgres/testversion/data/postgresql.conf | 550 | 8 | lc_monetary | C
|
/home/postgres/testversion/data/postgresql.conf | 551 | 9 | lc_numeric | C
|
/home/postgres/testversion/data/postgresql.conf | 552 | 10 | lc_time | C
|
/home/postgres/testversion/data/postgresql.conf | 555 | 11 | default_text_search_config | pg_catalog.english
|
/home/postgres/testversion/data/conf.d/foo.conf | 1 | 12 | work_mem | bogus
|setting could not be applied
(12 rows)
Invalid GUC variable name (which causes SIGHUP processing to be abandoned,
so we don't get as far as noticing work_mem = bogus):
sourcefile | sourceline | seqno | name | setting
| error
-------------------------------------------------+------------+-------+----------------------------+--------------------+--------------------------------------
/home/postgres/testversion/data/postgresql.conf | 64 | 1 | max_connections | 100
|
/home/postgres/testversion/data/postgresql.conf | 116 | 2 | shared_buffers | 128MB
|
/home/postgres/testversion/data/postgresql.conf | 131 | 3 | dynamic_shared_memory_type | posix
|
/home/postgres/testversion/data/postgresql.conf | 446 | 4 | log_timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 533 | 5 | datestyle | iso, mdy
|
/home/postgres/testversion/data/postgresql.conf | 535 | 6 | timezone | US/Eastern
|
/home/postgres/testversion/data/postgresql.conf | 548 | 7 | lc_messages | C
|
/home/postgres/testversion/data/postgresql.conf | 550 | 8 | lc_monetary | C
|
/home/postgres/testversion/data/postgresql.conf | 551 | 9 | lc_numeric | C
|
/home/postgres/testversion/data/postgresql.conf | 552 | 10 | lc_time | C
|
/home/postgres/testversion/data/postgresql.conf | 555 | 11 | default_text_search_config | pg_catalog.english
|
/home/postgres/testversion/data/conf.d/foo.conf | 1 | 12 | work_mem | bogus
|
/home/postgres/testversion/data/postgresql.conf | 628 | 13 | bad | worse
|unrecognized configuration parameter
(13 rows)
ISTM that this represents a quantum jump in the usefulness of the
pg_file_settings view for its ostensible purpose of diagnosing config-file
problems, so I would like to push forward with getting it done even though
we're on the verge of 9.5alpha1.
However there's a further tweak to the view that I'd like to think about
making. Once this is in and we make the originally-discussed change to
suppress application of duplicated GUC entries, it would be possible to
mark the ignored entries in the view, which would save people the effort
of figuring out manually which ones were ignored. The question is exactly
how to mark the ignored entries. A simple tweak would be to put something
in the "error" column, say "ignored because of later duplicate entry".
However, this would break the property that an entry in the error column
means there's something you'd better fix, which I think would be a useful
rule for nagios-like monitoring tools.
Another issue with the API as it stands here is that you can't easily
distinguish errors that caused the entire config file to be ignored from
those that only prevented application of one setting.
The best idea I have at the moment is to also add a boolean "applied"
column which is true if the entry was successfully applied. Errors that
result in the whole file being ignored would mean that *all* the entries
show applied = false. Otherwise, applied = false with nothing in the
error column would imply that the entry was ignored due to a later
duplicate. There are multiple other ways it could be done of course;
anyone want to lobby for some other design?
There is more that could be done with this basic idea; in particular,
it would be useful if set_config failures would be broken down in more
detail than "setting could not be applied". That would require somewhat
invasive refactoring though, and it would only be an incremental
improvement in usability, so I'm disinclined to tackle the point under
time pressure.
Comments, better ideas? Barring strong objections I'd like to get this
finished over the weekend.
regards, tom lane
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 5c0a965..887077f 100644
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
*************** static sigjmp_buf *GUC_flex_fatal_jmp;
*** 47,52 ****
--- 47,58 ----
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);
*************** ProcessConfigFile(GucContext context)
*** 115,126 ****
bool error = false;
bool apply = false;
int elevel;
const char *ConfFileWithError;
ConfigVariable *item,
*head,
*tail;
int i;
- int file_variables_count = 0;
/*
* Config files are processed on startup (by the postmaster only)
--- 121,132 ----
bool error = false;
bool apply = false;
int elevel;
+ MemoryContext caller_cxt;
const char *ConfFileWithError;
ConfigVariable *item,
*head,
*tail;
int i;
/*
* Config files are processed on startup (by the postmaster only)
*************** ProcessConfigFile(GucContext context)
*** 135,149 ****
*/
elevel = IsUnderPostmaster ? DEBUG2 : LOG;
/* 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))
{
/* Syntax error(s) detected in the file, so bail out */
error = true;
! goto cleanup_list;
}
/*
--- 141,179 ----
*/
elevel = IsUnderPostmaster ? DEBUG2 : LOG;
+ /*
+ * Do all the work of processing the config file within a private context.
+ * We will keep this context around until the next ProcessConfigFile call,
+ * so that the pg_file_settings view can inspect the ConfigVariable list.
+ * However, we can guarantee that any memory leaked in this code will not
+ * accumulate across repeated SIGHUP cycles.
+ *
+ * First, flush the old data if any.
+ */
+ current_config_variables = NULL;
+ if (current_config_context)
+ {
+ MemoryContextDelete(current_config_context);
+ current_config_context = NULL;
+ }
+
+ current_config_context = AllocSetContextCreate(TopMemoryContext,
+ "Config file processing",
+ ALLOCSET_DEFAULT_MINSIZE,
+ ALLOCSET_DEFAULT_MINSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
+ caller_cxt = MemoryContextSwitchTo(current_config_context);
+
/* Parse the main config file into a list of option names and values */
ConfFileWithError = ConfigFileName;
head = tail = NULL;
+ ConfigFileLineno = 0;
if (!ParseConfigFile(ConfigFileName, NULL, true, 0, elevel, &head, &tail))
{
/* Syntax error(s) detected in the file, so bail out */
error = true;
! goto bail_out;
}
/*
*************** ProcessConfigFile(GucContext context)
*** 154,166 ****
*/
if (DataDir)
{
if (!ParseConfigFile(PG_AUTOCONF_FILENAME, NULL, false, 0, elevel,
&head, &tail))
{
/* Syntax error(s) detected in the file, so bail out */
error = true;
ConfFileWithError = PG_AUTOCONF_FILENAME;
! goto cleanup_list;
}
}
else
--- 184,197 ----
*/
if (DataDir)
{
+ ConfigFileLineno = 0;
if (!ParseConfigFile(PG_AUTOCONF_FILENAME, NULL, false, 0, elevel,
&head, &tail))
{
/* Syntax error(s) detected in the file, so bail out */
error = true;
ConfFileWithError = PG_AUTOCONF_FILENAME;
! goto bail_out;
}
}
else
*************** ProcessConfigFile(GucContext context)
*** 173,200 ****
* 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;
! /* Prune all items except "data_directory" from the list */
! for (item = head; item;)
{
! 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;
}
/*
* Quick exit if data_directory is not present in file.
*
--- 204,226 ----
* will be read later. OTOH, since data_directory isn't allowed in the
* PG_AUTOCONF_FILENAME file, it will never be overwritten later.
*/
! ConfigVariable *newlist = NULL;
! /*
! * Prune all items except the last "data_directory" from the list. We
! * needn't bother freeing space, since the working context will be
! * cleaned up in the next call.
! */
! for (item = head; item; item = item->next)
{
! if (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.
*
*************** ProcessConfigFile(GucContext context)
*** 203,212 ****
* the config file.
*/
if (head == NULL)
! return;
}
/*
* Mark all extant GUC variables as not present in the config file.
* We need this so that we can tell below which ones have been removed
* from the file since we last processed it.
--- 229,244 ----
* the config file.
*/
if (head == NULL)
! goto bail_out;
}
/*
+ * Remember the raw data for possible examination via the pg_file_settings
+ * view. We want to do this even if an elog(ERROR) gets thrown below.
+ */
+ current_config_variables = head;
+
+ /*
* Mark all extant GUC variables as not present in the config file.
* We need this so that we can tell below which ones have been removed
* from the file since we last processed it.
*************** ProcessConfigFile(GucContext context)
*** 253,262 ****
errmsg("unrecognized configuration parameter \"%s\" in file \"%s\" line %u",
item->name,
item->filename, item->sourceline)));
error = true;
ConfFileWithError = item->filename;
}
- file_variables_count++;
}
/*
--- 285,294 ----
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;
}
}
/*
*************** ProcessConfigFile(GucContext context)
*** 264,270 ****
* any changes.
*/
if (error)
! goto cleanup_list;
/* Otherwise, set flag that we're beginning to apply changes */
apply = true;
--- 296,302 ----
* any changes.
*/
if (error)
! goto bail_out;
/* Otherwise, set flag that we're beginning to apply changes */
apply = true;
*************** ProcessConfigFile(GucContext context)
*** 289,294 ****
--- 321,327 ----
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed without restarting the server",
gconf->name)));
+ item->errmsg = pstrdup("parameter cannot be changed without restarting the server");
error = true;
continue;
}
*************** ProcessConfigFile(GucContext context)
*** 344,397 ****
}
/*
- * 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.
*/
for (item = head; item; item = item->next)
--- 377,382 ----
*************** ProcessConfigFile(GucContext context)
*** 432,437 ****
--- 417,423 ----
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 */
*************** ProcessConfigFile(GucContext context)
*** 453,459 ****
/* Remember when we last successfully loaded the config file. */
PgReloadTime = GetCurrentTimestamp();
! cleanup_list:
if (error)
{
/* During postmaster startup, any error is fatal */
--- 439,449 ----
/* Remember when we last successfully loaded the config file. */
PgReloadTime = GetCurrentTimestamp();
! bail_out:
! /* Successful or otherwise, remember the data collected from the files */
! current_config_variables = head;
!
! /* Report any problem */
if (error)
{
/* During postmaster startup, any error is fatal */
*************** ProcessConfigFile(GucContext context)
*** 475,485 ****
}
/*
! * Calling FreeConfigVariables() any earlier than this can cause problems,
! * because ConfFileWithError could be pointing to a string that will be
! * freed here.
*/
! FreeConfigVariables(head);
}
/*
--- 465,475 ----
}
/*
! * Return to caller's memory context. Don't delete current_config_context
! * now; we will do that when next called. This allows the ConfigVariables
! * list to be inspected after-the-fact.
*/
! MemoryContextSwitchTo(caller_cxt);
}
/*
*************** AbsoluteConfigLocation(const char *locat
*** 520,525 ****
--- 510,519 ----
* If "strict" is true, treat failure to open the config file as an error,
* otherwise just skip the file.
*
+ * Note that ConfigFileLineno should be considered to be an implicit parameter
+ * to this function: it is the line number within calling_file, if any, that
+ * should be fingered for error reporting purposes.
+ *
* 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.
*************** ParseConfigFile(const char *config_file,
*** 545,550 ****
--- 539,547 ----
(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, ConfigFileLineno,
+ head_p, tail_p);
return false;
}
*************** ParseConfigFile(const char *config_file,
*** 558,563 ****
--- 555,564 ----
(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, ConfigFileLineno,
+ head_p, tail_p);
OK = false;
}
else
*************** cleanup:
*** 580,585 ****
--- 581,613 ----
}
/*
+ * 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->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
* holds for flex 2.5.31 (earliest we support) and flex 2.5.35 (latest as of
*************** ParseConfigFp(FILE *fp, const char *conf
*** 641,647 ****
*/
elog(elevel, "%s at file \"%s\" line %u",
GUC_flex_fatal_errmsg, config_file, ConfigFileLineno);
!
OK = false;
goto cleanup;
}
--- 669,677 ----
*/
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;
}
*************** ParseConfigFp(FILE *fp, const char *conf
*** 697,702 ****
--- 727,735 ----
ConfigFileLineno++;
}
+ /* Temporarily decr ConfigFileLineno so it identifies correct line */
+ ConfigFileLineno--;
+
/* OK, process the option name and value */
if (guc_name_compare(opt_name, "include_dir") == 0)
{
*************** ParseConfigFp(FILE *fp, const char *conf
*** 709,715 ****
head_p, tail_p))
OK = false;
yy_switch_to_buffer(lex_buffer);
- ConfigFileLineno = save_ConfigFileLineno;
pfree(opt_name);
pfree(opt_value);
}
--- 742,747 ----
*************** ParseConfigFp(FILE *fp, const char *conf
*** 747,754 ****
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
item->filename = pstrdup(config_file);
! item->sourceline = ConfigFileLineno-1;
item->next = NULL;
if (*head_p == NULL)
*head_p = item;
--- 779,787 ----
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
+ item->errmsg = NULL;
item->filename = pstrdup(config_file);
! item->sourceline = ConfigFileLineno;
item->next = NULL;
if (*head_p == NULL)
*head_p = item;
*************** ParseConfigFp(FILE *fp, const char *conf
*** 757,762 ****
--- 790,798 ----
*tail_p = item;
}
+ /* Done processing line, restore ConfigFileLineno */
+ ConfigFileLineno++;
+
/* break out of loop if read EOF, else loop for next line */
if (token == 0)
break;
*************** ParseConfigFp(FILE *fp, const char *conf
*** 771,785 ****
--- 807,831 ----
/* 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++;
*************** cleanup:
*** 817,822 ****
--- 863,872 ----
/*
* Read and parse all config files in a subdirectory in alphabetical order
+ *
+ * Note that ConfigFileLineno should be considered to be an implicit parameter
+ * to this function: it is the line number within calling_file, if any, that
+ * should be fingered for error reporting purposes.
*/
bool
ParseConfigDirectory(const char *includedir,
*************** ParseConfigDirectory(const char *include
*** 841,846 ****
--- 891,900 ----
(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, ConfigFileLineno,
+ head_p, tail_p);
status = false;
goto cleanup;
}
*************** ParseConfigDirectory(const char *include
*** 897,902 ****
--- 951,960 ----
(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, ConfigFileLineno,
+ head_p, tail_p);
status = false;
goto cleanup;
}
*************** ParseConfigDirectory(const char *include
*** 908,914 ****
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))
{
status = false;
--- 966,972 ----
qsort(filenames, num_filenames, sizeof(char *), pg_qsort_strcmp);
for (i = 0; i < num_filenames; i++)
{
! if (!ParseConfigFile(filenames[i], calling_file, true,
depth, elevel, head_p, tail_p))
{
status = false;
*************** FreeConfigVariables(ConfigVariable *list
*** 949,957 ****
static void
FreeConfigVariable(ConfigVariable *item)
{
! pfree(item->name);
! pfree(item->value);
! pfree(item->filename);
pfree(item);
}
--- 1007,1020 ----
static void
FreeConfigVariable(ConfigVariable *item)
{
! 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);
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 230c5cc..cab8da0 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_generic **guc_varia
*** 3678,3702 ****
/* 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;
static bool guc_dirty; /* TRUE if need to do commit/abort work */
--- 3678,3695 ----
/* Current number of variables contained in the vector */
static int num_guc_variables;
/* Vector capacity */
static int size_guc_variables;
+ /*
+ * The raw data most recently fetched from the config file(s) is stored
+ * in the current_config_variables list, which is stored within the
+ * current_config_context memory context. We keep it around until the next
+ * attempt to read the config files, so that it can be displayed in the
+ * pg_file_settings view.
+ */
+ static ConfigVariable *current_config_variables = NULL;
+ static MemoryContext current_config_context = NULL;
static bool guc_dirty; /* TRUE if need to do commit/abort work */
*************** replace_auto_config_value(ConfigVariable
*** 6781,6786 ****
--- 6774,6780 ----
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->next = NULL;
*************** show_all_settings(PG_FUNCTION_ARGS)
*** 8181,8190 ****
/*
* 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.
*
* Note: no filtering is done here, instead we depend on the GRANT system
* to prevent unprivileged users from accessing this function or the view
--- 8175,8185 ----
/*
* show_all_file_settings
*
! * 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, 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
*************** show_all_settings(PG_FUNCTION_ARGS)
*** 8193,8203 ****
Datum
show_all_file_settings(PG_FUNCTION_ARGS)
{
! #define NUM_PG_FILE_SETTINGS_ATTS 5
FuncCallContext *funcctx;
TupleDesc tupdesc;
int call_cntr;
- int max_calls;
AttInMetadata *attinmeta;
MemoryContext oldcontext;
--- 8188,8198 ----
Datum
show_all_file_settings(PG_FUNCTION_ARGS)
{
! #define NUM_PG_FILE_SETTINGS_ATTS 6
FuncCallContext *funcctx;
TupleDesc tupdesc;
+ ConfigVariable *conf;
int call_cntr;
AttInMetadata *attinmeta;
MemoryContext oldcontext;
*************** show_all_file_settings(PG_FUNCTION_ARGS)
*** 8223,8260 ****
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "setting",
TEXTOID, -1, 0);
attinmeta = TupleDescGetAttInMetadata(tupdesc);
funcctx->attinmeta = attinmeta;
! funcctx->max_calls = num_guc_file_variables;
MemoryContextSwitchTo(oldcontext);
}
funcctx = SRF_PERCALL_SETUP();
call_cntr = funcctx->call_cntr;
- max_calls = funcctx->max_calls;
attinmeta = funcctx->attinmeta;
! if (call_cntr < max_calls)
{
char *values[NUM_PG_FILE_SETTINGS_ATTS];
HeapTuple tuple;
Datum result;
- ConfigFileVariable conf;
char buffer[12]; /* must be at least 12, per pg_ltoa */
! /* 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];
/* sourcefile */
! values[0] = conf.filename;
/* sourceline */
! pg_ltoa(conf.sourceline, buffer);
values[1] = pstrdup(buffer);
/* seqno */
--- 8218,8256 ----
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "setting",
TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 6, "error",
+ TEXTOID, -1, 0);
attinmeta = TupleDescGetAttInMetadata(tupdesc);
funcctx->attinmeta = attinmeta;
!
! /* We use user_fctx to point to next item to be returned */
! funcctx->user_fctx = (void *) current_config_variables;
!
MemoryContextSwitchTo(oldcontext);
}
funcctx = SRF_PERCALL_SETUP();
+ conf = (ConfigVariable *) funcctx->user_fctx;
call_cntr = funcctx->call_cntr;
attinmeta = funcctx->attinmeta;
! if (conf != NULL)
{
char *values[NUM_PG_FILE_SETTINGS_ATTS];
HeapTuple tuple;
Datum result;
char buffer[12]; /* must be at least 12, per pg_ltoa */
! /* Update pointer to next item to return */
! funcctx->user_fctx = (void *) conf->next;
/* sourcefile */
! values[0] = conf->filename;
/* sourceline */
! pg_ltoa(conf->sourceline, buffer);
values[1] = pstrdup(buffer);
/* seqno */
*************** show_all_file_settings(PG_FUNCTION_ARGS)
*** 8262,8271 ****
values[2] = pstrdup(buffer);
/* name */
! values[3] = conf.name;
/* setting */
! values[4] = conf.value;
/* build a tuple */
tuple = BuildTupleFromCStrings(attinmeta, values);
--- 8258,8270 ----
values[2] = pstrdup(buffer);
/* name */
! values[3] = conf->name;
/* setting */
! values[4] = conf->value;
!
! /* error */
! values[5] = conf->errmsg;
/* build a tuple */
tuple = BuildTupleFromCStrings(attinmeta, values);
*************** show_all_file_settings(PG_FUNCTION_ARGS)
*** 8275,8284 ****
SRF_RETURN_NEXT(funcctx, result);
}
! else
! {
! SRF_RETURN_DONE(funcctx);
! }
}
static char *
--- 8274,8281 ----
SRF_RETURN_NEXT(funcctx, result);
}
!
! SRF_RETURN_DONE(funcctx);
}
static char *
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 6b3d194..c03e2d3 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DATA(insert OID = 2078 ( set_config PG
*** 3071,3077 ****
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_ ));
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");
--- 3071,3077 ----
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,25}""{o,o,o,o,o,o}" "{sourcefile,sourceline,seqno,name,setting,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");
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index ffe1168..d7c286d 100644
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
*************** typedef enum
*** 121,133 ****
} GucSource;
/*
! * Parsing the configuration file will return a list of name-value pairs
! * with source location info.
*/
typedef struct ConfigVariable
{
char *name;
char *value;
char *filename;
int sourceline;
struct ConfigVariable *next;
--- 121,136 ----
} GucSource;
/*
! * 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.
*/
typedef struct ConfigVariable
{
char *name;
char *value;
+ char *errmsg;
char *filename;
int sourceline;
struct ConfigVariable *next;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 60c1f40..bcf24be 100644
*** a/src/test/regress/expected/rules.out
--- b/src/test/regress/expected/rules.out
*************** pg_file_settings| SELECT a.sourcefile,
*** 1316,1323 ****
a.sourceline,
a.seqno,
a.name,
! a.setting
! FROM pg_show_all_file_settings() a(sourcefile, sourceline, seqno, name, setting);
pg_group| SELECT pg_authid.rolname AS groname,
pg_authid.oid AS grosysid,
ARRAY( SELECT pg_auth_members.member
--- 1316,1324 ----
a.sourceline,
a.seqno,
a.name,
! a.setting,
! a.error
! FROM pg_show_all_file_settings() a(sourcefile, sourceline, seqno, name, setting, error);
pg_group| SELECT pg_authid.rolname AS groname,
pg_authid.oid AS grosysid,
ARRAY( SELECT pg_auth_members.member
pgsql-hackers by date: