Itagaki Takahiro <itagaki.takah...@gmail.com> writes:
> RECOVERY_COMMAND_FILE is opened twice in the patch. The first time
> is for checking the existence, and the second time is for parsing.
> Instead of the repeat, how about adding FILE* version of parser?
> It will be also called from ParseConfigFile() as a sub routine.
>
> bool ParseConfigFd(FILE *fd, const char *config_file, int depth, ...)
Something like the attached, version 5 of the patch? I've been using the
function name ParseConfigFp because the internal parameter was called fp
in the previous function body. I suppose that could easily be changed at
commit time if necessary.
> BTW, the parser supports "include" and "custom_variable_classes"
> not only for postgresql.conf but also for all files. Is it an
> intended behavior? I think they are harmless, so we don't have
> to change the codes; "include" might be useful even in recovery.conf,
> and "custom_variable_classes" will be "unrecognized recovery
> parameter" error after all.
Extensions will need the support for custom_variable_classes as it is
done now, and as you say, the recovery will just error out. You have to
clean your recovery.conf file then try again (I just tried and confirm).
I personally don't see any harm to have the features in all currently
known uses-cases, and I don't see any point in walking an extra mile
here to remove a feature in some cases.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 5024,5200 **** str_time(pg_time_t tnow)
}
/*
- * Parse one line from recovery.conf. 'cmdline' is the raw line from the
- * file. If the line is parsed successfully, returns true, false indicates
- * syntax error. On success, *key_p and *value_p are set to the parameter
- * name and value on the line, respectively. If the line is an empty line,
- * consisting entirely of whitespace and comments, function returns true
- * and *keyp_p and *value_p are set to NULL.
- *
- * The pointers returned in *key_p and *value_p point to an internal buffer
- * that is valid only until the next call of parseRecoveryCommandFile().
- */
- static bool
- parseRecoveryCommandFileLine(char *cmdline, char **key_p, char **value_p)
- {
- char *ptr;
- char *bufp;
- char *key;
- char *value;
- static char *buf = NULL;
-
- *key_p = *value_p = NULL;
-
- /*
- * Allocate the buffer on first use. It's used to hold both the parameter
- * name and value.
- */
- if (buf == NULL)
- buf = malloc(MAXPGPATH + 1);
- bufp = buf;
-
- /* Skip any whitespace at the beginning of line */
- for (ptr = cmdline; *ptr; ptr++)
- {
- if (!isspace((unsigned char) *ptr))
- break;
- }
- /* Ignore empty lines */
- if (*ptr == '\0' || *ptr == '#')
- return true;
-
- /* Read the parameter name */
- key = bufp;
- while (*ptr && !isspace((unsigned char) *ptr) &&
- *ptr != '=' && *ptr != '\'')
- *(bufp++) = *(ptr++);
- *(bufp++) = '\0';
-
- /* Skip to the beginning quote of the parameter value */
- ptr = strchr(ptr, '\'');
- if (!ptr)
- return false;
- ptr++;
-
- /* Read the parameter value to *bufp. Collapse any '' escapes as we go. */
- value = bufp;
- for (;;)
- {
- if (*ptr == '\'')
- {
- ptr++;
- if (*ptr == '\'')
- *(bufp++) = '\'';
- else
- {
- /* end of parameter */
- *bufp = '\0';
- break;
- }
- }
- else if (*ptr == '\0')
- return false; /* unterminated quoted string */
- else
- *(bufp++) = *ptr;
-
- ptr++;
- }
- *(bufp++) = '\0';
-
- /* Check that there's no garbage after the value */
- while (*ptr)
- {
- if (*ptr == '#')
- break;
- if (!isspace((unsigned char) *ptr))
- return false;
- ptr++;
- }
-
- /* Success! */
- *key_p = key;
- *value_p = value;
- return true;
- }
-
- /*
* See if there is a recovery command file (recovery.conf), and if so
* read in parameters for archive recovery and XLOG streaming.
*
! * XXX longer term intention is to expand this to
! * cater for additional parameters and controls
! * possibly use a flex lexer similar to the GUC one
*/
static void
readRecoveryCommandFile(void)
{
FILE *fd;
- char cmdline[MAXPGPATH];
TimeLineID rtli = 0;
bool rtliGiven = false;
! bool syntaxError = false;
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
{
if (errno == ENOENT)
! return; /* not there, so no archive recovery */
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not open recovery command file \"%s\": %m",
RECOVERY_COMMAND_FILE)));
}
! /*
! * Parse the file...
! */
! while (fgets(cmdline, sizeof(cmdline), fd) != NULL)
! {
! char *tok1;
! char *tok2;
! if (!parseRecoveryCommandFileLine(cmdline, &tok1, &tok2))
! {
! syntaxError = true;
! break;
! }
! if (tok1 == NULL)
! continue;
!
! if (strcmp(tok1, "restore_command") == 0)
{
! recoveryRestoreCommand = pstrdup(tok2);
ereport(DEBUG2,
(errmsg("restore_command = '%s'",
recoveryRestoreCommand)));
}
! else if (strcmp(tok1, "recovery_end_command") == 0)
{
! recoveryEndCommand = pstrdup(tok2);
ereport(DEBUG2,
(errmsg("recovery_end_command = '%s'",
recoveryEndCommand)));
}
! else if (strcmp(tok1, "archive_cleanup_command") == 0)
{
! archiveCleanupCommand = pstrdup(tok2);
ereport(DEBUG2,
(errmsg("archive_cleanup_command = '%s'",
archiveCleanupCommand)));
}
! else if (strcmp(tok1, "recovery_target_timeline") == 0)
{
rtliGiven = true;
! if (strcmp(tok2, "latest") == 0)
rtli = 0;
else
{
errno = 0;
! rtli = (TimeLineID) strtoul(tok2, NULL, 0);
if (errno == EINVAL || errno == ERANGE)
ereport(FATAL,
(errmsg("recovery_target_timeline is not a valid number: \"%s\"",
! tok2)));
}
if (rtli)
ereport(DEBUG2,
--- 5024,5095 ----
}
/*
* See if there is a recovery command file (recovery.conf), and if so
* read in parameters for archive recovery and XLOG streaming.
*
! * The file is parsed using the main configuration parser.
*/
static void
readRecoveryCommandFile(void)
{
FILE *fd;
TimeLineID rtli = 0;
bool rtliGiven = false;
! ConfigNameValuePair item, head, tail;
+ /*
+ * See if recovery.conf file is present
+ */
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
{
if (errno == ENOENT)
! return; /* not there, so no archive recovery */
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not open recovery command file \"%s\": %m",
RECOVERY_COMMAND_FILE)));
}
! if (!ParseConfigFp(fd, RECOVERY_COMMAND_FILE, NULL, 0, FATAL, &head, &tail))
! goto cleanup_list;
! for (item = head; item; item = item->next)
! {
! if (strcmp(item->name, "restore_command") == 0)
{
! recoveryRestoreCommand = pstrdup(item->value);
ereport(DEBUG2,
(errmsg("restore_command = '%s'",
recoveryRestoreCommand)));
}
! else if (strcmp(item->name, "recovery_end_command") == 0)
{
! recoveryEndCommand = pstrdup(item->value);
ereport(DEBUG2,
(errmsg("recovery_end_command = '%s'",
recoveryEndCommand)));
}
! else if (strcmp(item->name, "archive_cleanup_command") == 0)
{
! archiveCleanupCommand = pstrdup(item->value);
ereport(DEBUG2,
(errmsg("archive_cleanup_command = '%s'",
archiveCleanupCommand)));
}
! else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
rtliGiven = true;
! if (strcmp(item->value, "latest") == 0)
rtli = 0;
else
{
errno = 0;
! rtli = (TimeLineID) strtoul(item->value, NULL, 0);
if (errno == EINVAL || errno == ERANGE)
ereport(FATAL,
(errmsg("recovery_target_timeline is not a valid number: \"%s\"",
! item->value)));
}
if (rtli)
ereport(DEBUG2,
***************
*** 5203,5222 **** readRecoveryCommandFile(void)
ereport(DEBUG2,
(errmsg("recovery_target_timeline = latest")));
}
! else if (strcmp(tok1, "recovery_target_xid") == 0)
{
errno = 0;
! recoveryTargetXid = (TransactionId) strtoul(tok2, NULL, 0);
if (errno == EINVAL || errno == ERANGE)
ereport(FATAL,
(errmsg("recovery_target_xid is not a valid number: \"%s\"",
! tok2)));
ereport(DEBUG2,
(errmsg("recovery_target_xid = %u",
recoveryTargetXid)));
recoveryTarget = RECOVERY_TARGET_XID;
}
! else if (strcmp(tok1, "recovery_target_time") == 0)
{
/*
* if recovery_target_xid specified, then this overrides
--- 5098,5117 ----
ereport(DEBUG2,
(errmsg("recovery_target_timeline = latest")));
}
! else if (strcmp(item->name, "recovery_target_xid") == 0)
{
errno = 0;
! recoveryTargetXid = (TransactionId) strtoul(item->value, NULL, 0);
if (errno == EINVAL || errno == ERANGE)
ereport(FATAL,
(errmsg("recovery_target_xid is not a valid number: \"%s\"",
! item->value)));
ereport(DEBUG2,
(errmsg("recovery_target_xid = %u",
recoveryTargetXid)));
recoveryTarget = RECOVERY_TARGET_XID;
}
! else if (strcmp(item->name, "recovery_target_time") == 0)
{
/*
* if recovery_target_xid specified, then this overrides
***************
*** 5231,5274 **** readRecoveryCommandFile(void)
*/
recoveryTargetTime =
DatumGetTimestampTz(DirectFunctionCall3(timestamptz_in,
! CStringGetDatum(tok2),
ObjectIdGetDatum(InvalidOid),
Int32GetDatum(-1)));
ereport(DEBUG2,
(errmsg("recovery_target_time = '%s'",
timestamptz_to_str(recoveryTargetTime))));
}
! else if (strcmp(tok1, "recovery_target_inclusive") == 0)
{
/*
* does nothing if a recovery_target is not also set
*/
! if (!parse_bool(tok2, &recoveryTargetInclusive))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "recovery_target_inclusive")));
ereport(DEBUG2,
! (errmsg("recovery_target_inclusive = %s", tok2)));
}
! else if (strcmp(tok1, "standby_mode") == 0)
{
! if (!parse_bool(tok2, &StandbyMode))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "standby_mode")));
ereport(DEBUG2,
! (errmsg("standby_mode = '%s'", tok2)));
}
! else if (strcmp(tok1, "primary_conninfo") == 0)
{
! PrimaryConnInfo = pstrdup(tok2);
ereport(DEBUG2,
(errmsg("primary_conninfo = '%s'",
PrimaryConnInfo)));
}
! else if (strcmp(tok1, "trigger_file") == 0)
{
! TriggerFile = pstrdup(tok2);
ereport(DEBUG2,
(errmsg("trigger_file = '%s'",
TriggerFile)));
--- 5126,5169 ----
*/
recoveryTargetTime =
DatumGetTimestampTz(DirectFunctionCall3(timestamptz_in,
! CStringGetDatum(item->value),
ObjectIdGetDatum(InvalidOid),
Int32GetDatum(-1)));
ereport(DEBUG2,
(errmsg("recovery_target_time = '%s'",
timestamptz_to_str(recoveryTargetTime))));
}
! else if (strcmp(item->name, "recovery_target_inclusive") == 0)
{
/*
* does nothing if a recovery_target is not also set
*/
! if (!parse_bool(item->value, &recoveryTargetInclusive))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "recovery_target_inclusive")));
ereport(DEBUG2,
! (errmsg("recovery_target_inclusive = %s", item->value)));
}
! else if (strcmp(item->name, "standby_mode") == 0)
{
! if (!parse_bool(item->value, &StandbyMode))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "standby_mode")));
ereport(DEBUG2,
! (errmsg("standby_mode = '%s'", item->value)));
}
! else if (strcmp(item->name, "primary_conninfo") == 0)
{
! PrimaryConnInfo = pstrdup(item->value);
ereport(DEBUG2,
(errmsg("primary_conninfo = '%s'",
PrimaryConnInfo)));
}
! else if (strcmp(item->name, "trigger_file") == 0)
{
! TriggerFile = pstrdup(item->value);
ereport(DEBUG2,
(errmsg("trigger_file = '%s'",
TriggerFile)));
***************
*** 5276,5292 **** readRecoveryCommandFile(void)
else
ereport(FATAL,
(errmsg("unrecognized recovery parameter \"%s\"",
! tok1)));
}
- FreeFile(fd);
-
- if (syntaxError)
- ereport(FATAL,
- (errmsg("syntax error in recovery command file: %s",
- cmdline),
- errhint("Lines should have the format parameter = 'value'.")));
-
/*
* Check for compulsory parameters
*/
--- 5171,5179 ----
else
ereport(FATAL,
(errmsg("unrecognized recovery parameter \"%s\"",
! item->name)));
}
/*
* Check for compulsory parameters
*/
***************
*** 5332,5337 **** readRecoveryCommandFile(void)
--- 5219,5228 ----
recoveryTargetTLI = findNewestTimeLine(recoveryTargetTLI);
}
}
+
+ cleanup_list:
+ free_name_value_list(head);
+ FreeFile(fd);
}
/*
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
***************
*** 35,59 **** enum {
GUC_ERROR = 100
};
- struct name_value_pair
- {
- char *name;
- char *value;
- char *filename;
- int sourceline;
- struct name_value_pair *next;
- };
-
static unsigned int ConfigFileLineno;
/* flex fails to supply a prototype for yylex, so provide one */
int GUC_yylex(void);
- static bool ParseConfigFile(const char *config_file, const char *calling_file,
- int depth, int elevel,
- struct name_value_pair **head_p,
- struct name_value_pair **tail_p);
- static void free_name_value_list(struct name_value_pair * list);
static char *GUC_scanstr(const char *s);
%}
--- 35,45 ----
***************
*** 118,124 **** void
ProcessConfigFile(GucContext context)
{
int elevel;
! struct name_value_pair *item, *head, *tail;
char *cvc = NULL;
struct config_string *cvc_struct;
const char *envvar;
--- 104,110 ----
ProcessConfigFile(GucContext context)
{
int elevel;
! ConfigNameValuePair item, head, tail;
char *cvc = NULL;
struct config_string *cvc_struct;
const char *envvar;
***************
*** 356,400 **** ProcessConfigFile(GucContext context)
free(cvc);
}
-
/*
! * Read and parse a single configuration file. This function recurses
! * to handle "include" directives.
! *
! * Input parameters:
! * config_file: absolute or relative path of file to read
! * calling_file: absolute path of file containing the "include" directive,
! * or NULL at outer level (config_file must be absolute at outer level)
! * depth: recursion depth (used only to prevent infinite recursion)
! * elevel: error logging level determined by ProcessConfigFile()
! * Output parameters:
! * head_p, tail_p: head and tail of linked list of name/value pairs
! *
! * *head_p and *tail_p must be initialized to NULL before calling the outer
! * recursion level. On exit, they contain a list of name-value pairs read
! * from the input file(s).
! *
! * 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
! * its own state and release the name/value pairs list.
! *
! * Note: if elevel >= ERROR then an error will not return control to the
! * caller, and internal state such as open files will not be cleaned up.
! * This case occurs only during postmaster or standalone-backend startup,
! * where an error will lead to immediate process exit anyway; so there is
! * no point in contorting the code so it can clean up nicely.
*/
! static bool
ParseConfigFile(const char *config_file, const char *calling_file,
int depth, int elevel,
struct name_value_pair **head_p,
struct name_value_pair **tail_p)
{
bool OK = true;
- char abs_path[MAXPGPATH];
FILE *fp;
! YY_BUFFER_STATE lex_buffer;
! int token;
/*
* Reject too-deep include nesting depth. This is just a safety check
--- 342,360 ----
free(cvc);
}
/*
! * See next function for details. This one will just work with a config_file
! * name rather than an already opened File Descriptor
*/
! bool
ParseConfigFile(const char *config_file, const char *calling_file,
int depth, int elevel,
struct name_value_pair **head_p,
struct name_value_pair **tail_p)
{
bool OK = true;
FILE *fp;
! char abs_path[MAXPGPATH];
/*
* Reject too-deep include nesting depth. This is just a safety check
***************
*** 416,427 **** ParseConfigFile(const char *config_file, const char *calling_file,
*/
if (!is_absolute_path(config_file))
{
! Assert(calling_file != NULL);
! strlcpy(abs_path, calling_file, sizeof(abs_path));
! get_parent_directory(abs_path);
! join_path_components(abs_path, abs_path, config_file);
! canonicalize_path(abs_path);
! config_file = abs_path;
}
fp = AllocateFile(config_file, "r");
--- 376,398 ----
*/
if (!is_absolute_path(config_file))
{
! if (calling_file != NULL)
! {
! strlcpy(abs_path, calling_file, sizeof(abs_path));
! get_parent_directory(abs_path);
! join_path_components(abs_path, abs_path, config_file);
! canonicalize_path(abs_path);
! config_file = abs_path;
! }
! else
! {
! /*
! * calling_file is NULL, we make an absolute path from $PGDATA
! */
! join_path_components(abs_path, data_directory, config_file);
! canonicalize_path(abs_path);
! config_file = abs_path;
! }
}
fp = AllocateFile(config_file, "r");
***************
*** 434,439 **** ParseConfigFile(const char *config_file, const char *calling_file,
--- 405,456 ----
return false;
}
+ OK = ParseConfigFp(fp, config_file, calling_file,
+ depth, elevel, head_p, tail_p);
+
+ FreeFile(fp);
+
+ return OK;
+ }
+
+ /*
+ * Read and parse a single configuration file. This function recurses
+ * to handle "include" directives.
+ *
+ * Input parameters:
+ * fp: file pointer from AllocateFile for the configuration file to parse
+ * config_file: absolute or relative path of file to read
+ * calling_file: absolute path of file containing the "include" directive,
+ * or NULL at outer level (config_file must be absolute at outer level)
+ * depth: recursion depth (used only to prevent infinite recursion)
+ * elevel: error logging level determined by ProcessConfigFile()
+ * Output parameters:
+ * head_p, tail_p: head and tail of linked list of name/value pairs
+ *
+ * *head_p and *tail_p must be initialized to NULL before calling the outer
+ * recursion level. On exit, they contain a list of name-value pairs read
+ * from the input file(s).
+ *
+ * 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
+ * its own state and release the name/value pairs list.
+ *
+ * Note: if elevel >= ERROR then an error will not return control to the
+ * caller, and internal state such as open files will not be cleaned up.
+ * This case occurs only during postmaster or standalone-backend startup,
+ * where an error will lead to immediate process exit anyway; so there is
+ * no point in contorting the code so it can clean up nicely.
+ */
+ bool
+ ParseConfigFp(FILE *fp, const char *config_file, const char *calling_file,
+ int depth, int elevel,
+ struct name_value_pair **head_p,
+ struct name_value_pair **tail_p)
+ {
+ bool OK = true;
+ YY_BUFFER_STATE lex_buffer;
+ int token;
+
/*
* Parse
*/
***************
*** 579,585 **** ParseConfigFile(const char *config_file, const char *calling_file,
cleanup_exit:
yy_delete_buffer(lex_buffer);
- FreeFile(fp);
return OK;
}
--- 596,601 ----
***************
*** 587,593 **** cleanup_exit:
/*
* Free a list of name/value pairs, including the names and the values
*/
! static void
free_name_value_list(struct name_value_pair *list)
{
struct name_value_pair *item;
--- 603,609 ----
/*
* Free a list of name/value pairs, including the names and the values
*/
! void
free_name_value_list(struct name_value_pair *list)
{
struct name_value_pair *item;
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 403,408 **** int tcp_keepalives_interval;
--- 403,414 ----
int tcp_keepalives_count;
/*
+ * This is part of the dummy variables, but it needs to be exported so that
+ * guc-file.l can create $PGDATA/some.conf out of relative path.
+ */
+ char *data_directory;
+
+ /*
* These variables are all dummies that don't do anything, except in some
* cases provide the value for SHOW to display. The real state is elsewhere
* and is kept in sync by assign_hooks.
***************
*** 426,432 **** static char *timezone_string;
static char *log_timezone_string;
static char *timezone_abbreviations_string;
static char *XactIsoLevel_string;
- static char *data_directory;
static char *custom_variable_classes;
static int max_function_args;
static int max_index_keys;
--- 432,437 ----
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 17,23 ****
#include "tcop/dest.h"
#include "utils/array.h"
-
/*
* Certain options can only be set at certain times. The rules are
* like this:
--- 17,22 ----
***************
*** 97,102 **** typedef enum
--- 96,127 ----
} GucSource;
/*
+ * Parsing the configuation file will return a list of name-value pairs
+ */
+ struct name_value_pair
+ {
+ char *name;
+ char *value;
+ char *filename;
+ int sourceline;
+ struct name_value_pair *next;
+ } name_value_pair;
+
+ typedef struct name_value_pair *ConfigNameValuePair;
+
+ bool ParseConfigFile(const char *config_file, const char *calling_file,
+ int depth, int elevel,
+ ConfigNameValuePair *head_p,
+ ConfigNameValuePair *tail_p);
+
+ bool ParseConfigFp(FILE *fp, const char *config_file, const char *calling_file,
+ int depth, int elevel,
+ ConfigNameValuePair *head_p,
+ ConfigNameValuePair *tail_p);
+
+ void free_name_value_list(struct name_value_pair * list);
+
+ /*
* Enum values are made up of an array of name-value pairs
*/
struct config_enum_entry
***************
*** 187,192 **** extern int tcp_keepalives_idle;
--- 212,220 ----
extern int tcp_keepalives_interval;
extern int tcp_keepalives_count;
+ /* guc-file.l needs that to make absolute paths from relative ones */
+ extern char *data_directory;
+
extern void SetConfigOption(const char *name, const char *value,
GucContext context, GucSource source);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
