Thread: Allow commenting of variables in postgresql.conf to restore them to defaults
Allow commenting of variables in postgresql.conf to restore them to defaults
From
Zdenek Kotala
Date:
There is path implements following item from todo list: "Allow
commenting of variables in postgresql.conf to restore them to defaults".
Main idea is:
General config structure is extend with default_val attribute to keep
really default value. (There is small conflict - for string boot_val has same meaning).
During reconfiguration all values which has reset source equal with
PGC_S_FILE are revert back to really default values. New values from
configuration files are set after this step and commented variables stay
with default value.
Zdenek
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -r1.37 guc-file.l
115c115
< int elevel;
---
> int elevel, i;
116a117
> char *env;
145a147,182
> /* Revert all options with reset source PGC_S_FILE to default value.
> * This implementation is easier then implementing some change flag and verify
> * what really was commented out.
> * XXX When log_line_prefix is set in configuration file then log output
> * is not correct during this phase - prefix is revert to empty value.
> */
> for (i = 0; i < num_guc_variables; i++)
> {
> struct config_generic *gconf = guc_variables[i];
> if ( gconf->reset_source == PGC_S_FILE )
> {
> set_config_option(gconf->name, NULL, context,
> PGC_S_FILE, false, true);
> }
> }
>
> /* Revert to environment variable. PGPORT is ignored, because it cannot be
> * set in running state. PGC_S_FILE is used instead PGC_S_ENV so as
> * set_config_option can override previous defined option in config file.
> * If these options are still in config file They will be overridden in
> * the following step.
> */
> env = getenv("PGDATESTYLE");
> if (env != NULL)
> {
> set_config_option("datestyle", env, context,
> PGC_S_FILE, false, true);
> }
>
> env = getenv("PGCLIENTENCODING");
> if (env != NULL)
> {
> set_config_option("client_encoding", env, context,
> PGC_S_FILE, false, true);
> }
>
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.319
diff -r1.319 guc.c
2651c2651
< if (!(*conf->assign_hook) (conf->reset_val, true,
---
> if (!(*conf->assign_hook) (conf->default_val, true,
2654,2655c2654,2655
< conf->gen.name, (int) conf->reset_val);
< *conf->variable = conf->reset_val;
---
> conf->gen.name, (int) conf->default_val);
> *conf->variable = conf->reset_val = conf->default_val;
2665c2665
< if (!(*conf->assign_hook) (conf->reset_val, true,
---
> if (!(*conf->assign_hook) (conf->default_val, true,
2668,2669c2668,2669
< conf->gen.name, conf->reset_val);
< *conf->variable = conf->reset_val;
---
> conf->gen.name, conf->default_val);
> *conf->variable = conf->reset_val = conf->default_val;
2679c2679
< if (!(*conf->assign_hook) (conf->reset_val, true,
---
> if (!(*conf->assign_hook) (conf->default_val, true,
2682,2683c2682,2683
< conf->gen.name, conf->reset_val);
< *conf->variable = conf->reset_val;
---
> conf->gen.name, conf->default_val);
> *conf->variable = conf->reset_val = conf->default_val;
3650c3650
< if (changeVal && !is_newvalue_equal(record, value))
---
> if (changeVal && value != NULL && !is_newvalue_equal(record, value))
3726c3726
< makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
---
> makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
3769,3770c3769,3781
< newval = conf->reset_val;
< source = conf->gen.reset_source;
---
> /* Revert value to default if source is configuration file. It is used when
> * configuration parameter is removed/commented out in the config file. Else
> * RESET or SET TO DEFAULT command is called and reset_val is used.
> */
> if( source == PGC_S_FILE )
> {
> newval = conf->default_val;
> }
> else
> {
> newval = conf->reset_val;
> source = conf->gen.reset_source;
> }
3853,3854c3864,3876
< newval = conf->reset_val;
< source = conf->gen.reset_source;
---
> /* Revert value to default if source is configuration file. It is used when
> * configuration parameter is removed/commented out in the config file. Else
> * RESET or SET TO DEFAULT command is called and reset_val is used.
> */
> if( source == PGC_S_FILE )
> {
> newval = conf->default_val;
> }
> else
> {
> newval = conf->reset_val;
> source = conf->gen.reset_source;
> }
3937,3938c3959,3971
< newval = conf->reset_val;
< source = conf->gen.reset_source;
---
> /* Revert value to default if source is configuration file. It is used when
> * configuration parameter is removed/commented out in the config file. Else
> * RESET or SET TO DEFAULT command is called and reset_val is used.
> */
> if( source == PGC_S_FILE )
> {
> newval = conf->default_val;
> }
> else
> {
> newval = conf->reset_val;
> source = conf->gen.reset_source;
> }
4011a4045,4058
> else if (source == PGC_S_FILE)
> {
> /* Revert value to default when item is removed from config file. */
> if ( conf->boot_val != NULL )
> {
> newval = guc_strdup(elevel, conf->boot_val);
> if (newval == NULL)
> return false;
> }
> else
> {
> return false;
> }
> }
5113a5161,5165
> if( !newvalue )
> {
> return false;
> }
>
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.22
diff -r1.22 guc_tables.h
145c145
< bool reset_val;
---
> bool default_val;
149a150
> bool reset_val;
158c159
< int reset_val;
---
> int default_val;
164a166
> int reset_val;
173c175
< double reset_val;
---
> double default_val;
179a182,183
> double reset_val;
>
Zdenek Kotala wrote: > There is path implements following item from todo list: "Allow > commenting of variables in postgresql.conf to restore them to defaults". > Main idea is: > > General config structure is extend with default_val attribute to keep > really default value. (There is small conflict - for string boot_val > has same meaning). > During reconfiguration all values which has reset source equal with > PGC_S_FILE are revert back to really default values. New values from > configuration files are set after this step and commented variables > stay with default value. > Please resubmit your patch as a context diff, as documented here: http://www.postgresql.org/docs/faqs.FAQ_DEV.html#item1.5 cheers andrew
Andrew Dunstan wrote: > Zdenek Kotala wrote: >> There is path implements following item from todo list: "Allow >> commenting of variables in postgresql.conf to restore them to defaults". >> Main idea is: >> >> General config structure is extend with default_val attribute to keep >> really default value. (There is small conflict - for string boot_val >> has same meaning). >> During reconfiguration all values which has reset source equal with >> PGC_S_FILE are revert back to really default values. New values from >> configuration files are set after this step and commented variables >> stay with default value. >> > > Please resubmit your patch as a context diff, as documented here: > http://www.postgresql.org/docs/faqs.FAQ_DEV.html#item1.5 > > cheers > > andrew I am sorry. Here it is: Index: src/backend/utils/misc/guc-file.l =================================================================== RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v retrieving revision 1.37 diff -c -r1.37 guc-file.l *** src/backend/utils/misc/guc-file.l 7 Mar 2006 01:03:12 -0000 1.37 --- src/backend/utils/misc/guc-file.l 24 May 2006 14:10:12 -0000 *************** *** 112,119 **** void ProcessConfigFile(GucContext context) { ! int elevel; struct name_value_pair *item, *head, *tail; Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP); --- 112,120 ---- void ProcessConfigFile(GucContext context) { ! int elevel, i; struct name_value_pair *item, *head, *tail; + char *env; Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP); *************** *** 143,148 **** --- 144,185 ---- goto cleanup_list; } + /* Revert all options with reset source PGC_S_FILE to default value. + * This implementation is easier then iplementing some change flag and verify + * what realy was commented out. + * XXX When log_line_prefix is set in configuration file then log output + * is not correct during this phase - prefix is revert to empty value. + */ + for (i = 0; i < num_guc_variables; i++) + { + struct config_generic *gconf = guc_variables[i]; + if ( gconf->reset_source == PGC_S_FILE ) + { + set_config_option(gconf->name, NULL, context, + PGC_S_FILE, false, true); + } + } + + /* Revert to environment variable. PGPORT is ignored, because it cannot be + * set in running state. PGC_S_FILE is used instead PGC_S_ENV so as + * set_config_option can override previous defined option in config file. + * If these options are still in config file They will be overriden in + * the following step. + */ + env = getenv("PGDATESTYLE"); + if (env != NULL) + { + set_config_option("datestyle", env, context, + PGC_S_FILE, false, true); + } + + env = getenv("PGCLIENTENCODING"); + if (env != NULL) + { + set_config_option("client_encoding", env, context, + PGC_S_FILE, false, true); + } + /* If we got here all the options checked out okay, so apply them. */ for (item = head; item; item = item->next) { Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.319 diff -c -r1.319 guc.c *** src/backend/utils/misc/guc.c 11 May 2006 19:15:35 -0000 1.319 --- src/backend/utils/misc/guc.c 24 May 2006 14:10:12 -0000 *************** *** 2648,2658 **** struct config_bool *conf = (struct config_bool *) gconf; if (conf->assign_hook) ! if (!(*conf->assign_hook) (conf->reset_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %d", ! conf->gen.name, (int) conf->reset_val); ! *conf->variable = conf->reset_val; break; } case PGC_INT: --- 2648,2658 ---- struct config_bool *conf = (struct config_bool *) gconf; if (conf->assign_hook) ! if (!(*conf->assign_hook) (conf->default_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %d", ! conf->gen.name, (int) conf->default_val); ! *conf->variable = conf->reset_val = conf->default_val; break; } case PGC_INT: *************** *** 2662,2672 **** Assert(conf->reset_val >= conf->min); Assert(conf->reset_val <= conf->max); if (conf->assign_hook) ! if (!(*conf->assign_hook) (conf->reset_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %d", ! conf->gen.name, conf->reset_val); ! *conf->variable = conf->reset_val; break; } case PGC_REAL: --- 2662,2672 ---- Assert(conf->reset_val >= conf->min); Assert(conf->reset_val <= conf->max); if (conf->assign_hook) ! if (!(*conf->assign_hook) (conf->default_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %d", ! conf->gen.name, conf->default_val); ! *conf->variable = conf->reset_val = conf->default_val; break; } case PGC_REAL: *************** *** 2676,2686 **** Assert(conf->reset_val >= conf->min); Assert(conf->reset_val <= conf->max); if (conf->assign_hook) ! if (!(*conf->assign_hook) (conf->reset_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %g", ! conf->gen.name, conf->reset_val); ! *conf->variable = conf->reset_val; break; } case PGC_STRING: --- 2676,2686 ---- Assert(conf->reset_val >= conf->min); Assert(conf->reset_val <= conf->max); if (conf->assign_hook) ! if (!(*conf->assign_hook) (conf->default_val, true, PGC_S_DEFAULT)) elog(FATAL, "failed to initialize %s to %g", ! conf->gen.name, conf->default_val); ! *conf->variable = conf->reset_val = conf->default_val; break; } case PGC_STRING: *************** *** 3647,3653 **** case PGC_POSTMASTER: if (context == PGC_SIGHUP) { ! if (changeVal && !is_newvalue_equal(record, value)) ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored", --- 3647,3653 ---- case PGC_POSTMASTER: if (context == PGC_SIGHUP) { ! if (changeVal && value != NULL && !is_newvalue_equal(record, value)) ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored", *************** *** 3723,3729 **** * Should we set reset/stacked values? (If so, the behavior is not * transactional.) */ ! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL); /* * Ignore attempted set if overridden by previously processed setting. --- 3723,3729 ---- * Should we set reset/stacked values? (If so, the behavior is not * transactional.) */ ! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE); /* * Ignore attempted set if overridden by previously processed setting. *************** *** 3766,3773 **** } else { ! newval = conf->reset_val; ! source = conf->gen.reset_source; } if (conf->assign_hook) --- 3766,3784 ---- } else { ! /* Revert value to default if source is configuration file. It is used when ! * configuration parameter is removed/commented out in the config file. Else ! * RESET or SET TO DEFAULT command is called and reset_val is used. ! */ ! if( source == PGC_S_FILE ) ! { ! newval = conf->default_val; ! } ! else ! { ! newval = conf->reset_val; ! source = conf->gen.reset_source; ! } } if (conf->assign_hook) *************** *** 3850,3857 **** } else { ! newval = conf->reset_val; ! source = conf->gen.reset_source; } if (conf->assign_hook) --- 3861,3879 ---- } else { ! /* Revert value to default if source is configuration file. It is used when ! * configuration parameter is removed/commented out in the config file. Else ! * RESET or SET TO DEFAULT command is called and reset_val is used. ! */ ! if( source == PGC_S_FILE ) ! { ! newval = conf->default_val; ! } ! else ! { ! newval = conf->reset_val; ! source = conf->gen.reset_source; ! } } if (conf->assign_hook) *************** *** 3934,3941 **** } else { ! newval = conf->reset_val; ! source = conf->gen.reset_source; } if (conf->assign_hook) --- 3956,3974 ---- } else { ! /* Revert value to default if source is configuration file. It is used when ! * configuration parameter is removed/commented out in the config file. Else ! * RESET or SET TO DEFAULT command is called and reset_val is used. ! */ ! if( source == PGC_S_FILE ) ! { ! newval = conf->default_val; ! } ! else ! { ! newval = conf->reset_val; ! source = conf->gen.reset_source; ! } } if (conf->assign_hook) *************** *** 4009,4014 **** --- 4042,4061 ---- if (conf->gen.flags & GUC_IS_NAME) truncate_identifier(newval, strlen(newval), true); } + else if (source == PGC_S_FILE) + { + /* Revert value to default when item is removed from config file. */ + if ( conf->boot_val != NULL ) + { + newval = guc_strdup(elevel, conf->boot_val); + if (newval == NULL) + return false; + } + else + { + return false; + } + } else if (conf->reset_val) { /* *************** *** 5111,5116 **** --- 5158,5168 ---- static bool is_newvalue_equal(struct config_generic *record, const char *newvalue) { + if( !newvalue ) + { + return false; + } + switch (record->vartype) { case PGC_BOOL: Index: src/include/utils/guc_tables.h =================================================================== RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v retrieving revision 1.22 diff -c -r1.22 guc_tables.h *** src/include/utils/guc_tables.h 5 Mar 2006 15:59:07 -0000 1.22 --- src/include/utils/guc_tables.h 24 May 2006 14:10:13 -0000 *************** *** 142,152 **** /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ bool *variable; ! bool reset_val; GucBoolAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ bool tentative_val; }; struct config_int --- 142,153 ---- /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ bool *variable; ! bool default_val; GucBoolAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ bool tentative_val; + bool reset_val; }; struct config_int *************** *** 155,167 **** /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ int *variable; ! int reset_val; int min; int max; GucIntAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ int tentative_val; }; struct config_real --- 156,169 ---- /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ int *variable; ! int default_val; int min; int max; GucIntAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ int tentative_val; + int reset_val; }; struct config_real *************** *** 170,182 **** /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ double *variable; ! double reset_val; double min; double max; GucRealAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ double tentative_val; }; struct config_string --- 172,186 ---- /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ double *variable; ! double default_val; double min; double max; GucRealAssignHook assign_hook; GucShowHook show_hook; /* variable fields, initialized at runtime: */ double tentative_val; + double reset_val; + }; struct config_string
Zdenek Kotala wrote: > Andrew Dunstan wrote: > >Zdenek Kotala wrote: > >>There is path implements following item from todo list: "Allow > >>commenting of variables in postgresql.conf to restore them to defaults". > >>Main idea is: > >> > >>General config structure is extend with default_val attribute to keep > >>really default value. (There is small conflict - for string boot_val > >>has same meaning). > >>During reconfiguration all values which has reset source equal with > >>PGC_S_FILE are revert back to really default values. New values from > >>configuration files are set after this step and commented variables > >>stay with default value. > >> > > > >Please resubmit your patch as a context diff, as documented here: > >http://www.postgresql.org/docs/faqs.FAQ_DEV.html#item1.5 > I am sorry. Here it is: Please resubmit as an attachment, or disallow your mail client from munging whitespace ... I see wrapped words here, and apparently tab expansion as well. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote:
> Zdenek Kotala wrote:
>
>>> Zdenek Kotala wrote:
>>>
>>>> There is path implements following item from todo list: "Allow
>>>> commenting of variables in postgresql.conf to restore them to defaults".
>>>> Main idea is:
>>>>
>>>> General config structure is extend with default_val attribute to keep
>>>> really default value. (There is small conflict - for string boot_val
>>>> has same meaning).
>>>> During reconfiguration all values which has reset source equal with
>>>> PGC_S_FILE are revert back to really default values. New values from
>>>> configuration files are set after this step and commented variables
>>>> stay with default value.
>>>>
>>>>
>>>
>
> Please resubmit as an attachment, or disallow your mail client from
> munging whitespace ... I see wrapped words here, and apparently tab
> expansion as well.
>
>
OK. Here is patch like attachment.
Zdenek
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -c -r1.37 guc-file.l
*** src/backend/utils/misc/guc-file.l 7 Mar 2006 01:03:12 -0000 1.37
--- src/backend/utils/misc/guc-file.l 24 May 2006 14:10:12 -0000
***************
*** 112,119 ****
void
ProcessConfigFile(GucContext context)
{
! int elevel;
struct name_value_pair *item, *head, *tail;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 112,120 ----
void
ProcessConfigFile(GucContext context)
{
! int elevel, i;
struct name_value_pair *item, *head, *tail;
+ char *env;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
***************
*** 143,148 ****
--- 144,185 ----
goto cleanup_list;
}
+ /* Revert all options with reset source PGC_S_FILE to default value.
+ * This implementation is easier then iplementing some change flag and verify
+ * what realy was commented out.
+ * XXX When log_line_prefix is set in configuration file then log output
+ * is not correct during this phase - prefix is revert to empty value.
+ */
+ for (i = 0; i < num_guc_variables; i++)
+ {
+ struct config_generic *gconf = guc_variables[i];
+ if ( gconf->reset_source == PGC_S_FILE )
+ {
+ set_config_option(gconf->name, NULL, context,
+ PGC_S_FILE, false, true);
+ }
+ }
+
+ /* Revert to environment variable. PGPORT is ignored, because it cannot be
+ * set in running state. PGC_S_FILE is used instead PGC_S_ENV so as
+ * set_config_option can override previous defined option in config file.
+ * If these options are still in config file They will be overriden in
+ * the following step.
+ */
+ env = getenv("PGDATESTYLE");
+ if (env != NULL)
+ {
+ set_config_option("datestyle", env, context,
+ PGC_S_FILE, false, true);
+ }
+
+ env = getenv("PGCLIENTENCODING");
+ if (env != NULL)
+ {
+ set_config_option("client_encoding", env, context,
+ PGC_S_FILE, false, true);
+ }
+
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.319
diff -c -r1.319 guc.c
*** src/backend/utils/misc/guc.c 11 May 2006 19:15:35 -0000 1.319
--- src/backend/utils/misc/guc.c 24 May 2006 14:10:12 -0000
***************
*** 2648,2658 ****
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_INT:
--- 2648,2658 ----
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->default_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->default_val);
! *conf->variable = conf->reset_val = conf->default_val;
break;
}
case PGC_INT:
***************
*** 2662,2672 ****
Assert(conf->reset_val >= conf->min);
Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_REAL:
--- 2662,2672 ----
Assert(conf->reset_val >= conf->min);
Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->default_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->default_val);
! *conf->variable = conf->reset_val = conf->default_val;
break;
}
case PGC_REAL:
***************
*** 2676,2686 ****
Assert(conf->reset_val >= conf->min);
Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_STRING:
--- 2676,2686 ----
Assert(conf->reset_val >= conf->min);
Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->default_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->default_val);
! *conf->variable = conf->reset_val = conf->default_val;
break;
}
case PGC_STRING:
***************
*** 3647,3653 ****
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
! if (changeVal && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
--- 3647,3653 ----
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
! if (changeVal && value != NULL && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
***************
*** 3723,3729 ****
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
/*
* Ignore attempted set if overridden by previously processed setting.
--- 3723,3729 ----
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
/*
* Ignore attempted set if overridden by previously processed setting.
***************
*** 3766,3773 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3766,3784 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->default_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3850,3857 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3861,3879 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->default_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3934,3941 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3956,3974 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->default_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 4009,4014 ****
--- 4042,4061 ----
if (conf->gen.flags & GUC_IS_NAME)
truncate_identifier(newval, strlen(newval), true);
}
+ else if (source == PGC_S_FILE)
+ {
+ /* Revert value to default when item is removed from config file. */
+ if ( conf->boot_val != NULL )
+ {
+ newval = guc_strdup(elevel, conf->boot_val);
+ if (newval == NULL)
+ return false;
+ }
+ else
+ {
+ return false;
+ }
+ }
else if (conf->reset_val)
{
/*
***************
*** 5111,5116 ****
--- 5158,5168 ----
static bool
is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ if( !newvalue )
+ {
+ return false;
+ }
+
switch (record->vartype)
{
case PGC_BOOL:
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.22
diff -c -r1.22 guc_tables.h
*** src/include/utils/guc_tables.h 5 Mar 2006 15:59:07 -0000 1.22
--- src/include/utils/guc_tables.h 24 May 2006 14:10:13 -0000
***************
*** 142,152 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool reset_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
};
struct config_int
--- 142,153 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool default_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
+ bool reset_val;
};
struct config_int
***************
*** 155,167 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int reset_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
};
struct config_real
--- 156,169 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int default_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
+ int reset_val;
};
struct config_real
***************
*** 170,182 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double reset_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
};
struct config_string
--- 172,186 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double default_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
+ double reset_val;
+
};
struct config_string
Zdenek,
On Wed, May 24, 2006 at 04:27:01PM +0200, Zdenek Kotala wrote:
> >>>>General config structure is extend with default_val attribute to keep
> >>>>really default value. (There is small conflict - for string boot_val
> >>>>has same meaning).
> >>>>During reconfiguration all values which has reset source equal with
> >>>>PGC_S_FILE are revert back to really default values. New values from
> >>>>configuration files are set after this step and commented variables
> >>>>stay with default value.
Three points after a quick test:
- please compile with --enable-cassert, there are wrong assertions in your
code (you might just have to move some lines, there is an Assert() and an
assignment thereafter)
- changing a PGC_POSTMASTER should show a message:
=> parameter \"%s\" cannot be changed after server start;
configuration file change ignored
This seems to not show up anymore with your patch.
- with the same reasoning, I think it's a good idea to display a message
about which option falls back to its default value.
Joachim
Joachim, thanks for your comments. I am working on them. Zdenek Joachim Wieland wrote: > Zdenek, > > Three points after a quick test: > > - please compile with --enable-cassert, there are wrong assertions in your > code (you might just have to move some lines, there is an Assert() and an > assignment thereafter) > > - changing a PGC_POSTMASTER should show a message: > => parameter \"%s\" cannot be changed after server start; > configuration file change ignored > This seems to not show up anymore with your patch. > > - with the same reasoning, I think it's a good idea to display a message > about which option falls back to its default value. > > > Joachim > >
There is second version of patch. I made following changes:
- fix problem with assertion
- use boot_val instead default_val for all configuration data types
- add GUC_JUST_RELOAD status to track what is in configuration file or not
- revert only commented out variables
- add message what is revert to boot value
Joachim, could you explain me second point? I cannot determine
described problem. By my opinion my patch does not change this behavior.
Zdenek
Joachim Wieland wrote:
> Zdenek,
>
> Three points after a quick test:
>
> - please compile with --enable-cassert, there are wrong assertions in your
> code (you might just have to move some lines, there is an Assert() and an
> assignment thereafter)
>
> - changing a PGC_POSTMASTER should show a message:
> => parameter \"%s\" cannot be changed after server start;
> configuration file change ignored
> This seems to not show up anymore with your patch.
>
> - with the same reasoning, I think it's a good idea to display a message
> about which option falls back to its default value.
>
>
> Joachim
>
>
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -c -r1.37 guc-file.l
*** src/backend/utils/misc/guc-file.l 7 Mar 2006 01:03:12 -0000 1.37
--- src/backend/utils/misc/guc-file.l 31 May 2006 16:04:06 -0000
***************
*** 112,119 ****
void
ProcessConfigFile(GucContext context)
{
! int elevel;
struct name_value_pair *item, *head, *tail;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 112,120 ----
void
ProcessConfigFile(GucContext context)
{
! int elevel, i;
struct name_value_pair *item, *head, *tail;
+ char *env;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
***************
*** 150,155 ****
--- 151,205 ----
PGC_S_FILE, false, true);
}
+ if( context == PGC_SIGHUP)
+ {
+ /* Revert all "untouched" options with reset source PGC_S_FILE to
+ * default/boot value.
+ */
+ for (i = 0; i < num_guc_variables; i++)
+ {
+ struct config_generic *gconf = guc_variables[i];
+ if ( (gconf->reset_source == PGC_S_FILE) && ( (gconf->status & GUC_JUST_RELOAD) == 0) )
+ {
+ if( set_config_option(gconf->name, NULL, context,
+ PGC_S_FILE, false, true) )
+ {
+ GucStack *stack;
+ /* set correctly source */
+ gconf->reset_source = PGC_S_DEFAULT;
+ for (stack = gconf->stack; stack; stack = stack->prev)
+ {
+ if (stack->source == PGC_S_FILE)
+ {
+ stack->source = PGC_S_DEFAULT;
+ }
+ }
+
+ ereport(elevel,
+ (errcode(ERRCODE_SUCCESSFUL_COMPLETION),
+ errmsg("Configuration option %s has been revert to the boot value.", gconf->name)));
+ }
+ }
+ gconf->status &= ~GUC_JUST_RELOAD;
+ }
+
+ /* Revert to environment variable. PGPORT is ignored, because it cannot be
+ * set in running state.
+ */
+ env = getenv("PGDATESTYLE");
+ if (env != NULL)
+ {
+ set_config_option("datestyle", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+
+ env = getenv("PGCLIENTENCODING");
+ if (env != NULL)
+ {
+ set_config_option("client_encoding", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+ }
cleanup_list:
free_name_value_list(head);
}
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.319
diff -c -r1.319 guc.c
*** src/backend/utils/misc/guc.c 11 May 2006 19:15:35 -0000 1.319
--- src/backend/utils/misc/guc.c 31 May 2006 16:04:07 -0000
***************
*** 2648,2686 ****
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_STRING:
--- 2648,2686 ----
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_STRING:
***************
*** 3133,3139 ****
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status;
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
--- 3133,3139 ----
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status & (~GUC_JUST_RELOAD);
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
***************
*** 3647,3658 ****
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
! if (changeVal && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
name)));
!
return true;
}
if (context != PGC_POSTMASTER)
--- 3647,3658 ----
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
! if (changeVal && value != NULL && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
name)));
! record->status |= GUC_JUST_RELOAD;
return true;
}
if (context != PGC_POSTMASTER)
***************
*** 3693,3699 ****
--- 3693,3702 ----
* backend start.
*/
if (IsUnderPostmaster)
+ {
+ record->status |= GUC_JUST_RELOAD;
return true;
+ }
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
***************
*** 3723,3729 ****
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
/*
* Ignore attempted set if overridden by previously processed setting.
--- 3726,3732 ----
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
/*
* Ignore attempted set if overridden by previously processed setting.
***************
*** 3766,3773 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3769,3787 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3850,3857 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3864,3882 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3934,3941 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3959,3977 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 4009,4014 ****
--- 4045,4064 ----
if (conf->gen.flags & GUC_IS_NAME)
truncate_identifier(newval, strlen(newval), true);
}
+ else if (source == PGC_S_FILE)
+ {
+ /* Revert value to default when item is removed from config file. */
+ if ( conf->boot_val != NULL )
+ {
+ newval = guc_strdup(elevel, conf->boot_val);
+ if (newval == NULL)
+ return false;
+ }
+ else
+ {
+ return false;
+ }
+ }
else if (conf->reset_val)
{
/*
***************
*** 4117,4122 ****
--- 4167,4175 ----
if (changeVal && (record->flags & GUC_REPORT))
ReportGUCOption(record);
+ if(changeVal && record->source == PGC_S_FILE && value != NULL)
+ record->status |= GUC_JUST_RELOAD;
+
return true;
}
***************
*** 5111,5116 ****
--- 5164,5174 ----
static bool
is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ if( !newvalue )
+ {
+ return false;
+ }
+
switch (record->vartype)
{
case PGC_BOOL:
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.22
diff -c -r1.22 guc_tables.h
*** src/include/utils/guc_tables.h 5 Mar 2006 15:59:07 -0000 1.22
--- src/include/utils/guc_tables.h 31 May 2006 16:04:07 -0000
***************
*** 132,138 ****
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
!
/* GUC records for specific variable types */
--- 132,138 ----
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
! #define GUC_JUST_RELOAD 0x0008 /* value is just reload from configuration file */
/* GUC records for specific variable types */
***************
*** 142,152 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool reset_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
};
struct config_int
--- 142,153 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool boot_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
+ bool reset_val;
};
struct config_int
***************
*** 155,167 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int reset_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
};
struct config_real
--- 156,169 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int boot_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
+ int reset_val;
};
struct config_real
***************
*** 170,182 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double reset_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
};
struct config_string
--- 172,186 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double boot_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
+ double reset_val;
+
};
struct config_string
Zdenek, On Wed, May 31, 2006 at 06:13:04PM +0200, Zdenek Kotala wrote: > Joachim, could you explain me second point? I cannot determine > described problem. By my opinion my patch does not change this behavior. I guess what I saw was another phenomenon: I do the following: - vi postgresql.conf => "allow_system_table_mods = true" - start postmaster - vi postgresql.conf => "# allow_system_table_mods = true" (commented) - killall -HUP postmaster Then I get _no_ message. After another killall -HUP I do indeed get a message. So I don't get it just for the first time which is strange, do you see that as well? However the message I get is that it got reset to its default value which is wrong because its a PGC_POSTMASTER variable that can only be set at server start (set_config_option() returns true in this case as well). Consequently I expect to get it for every other signal I send (because the old value is still active and differs from what is in the configuration file). Joachim
Thanks, You have right. It is problem with silent skip some option in
set_config_option function when PGC_SIGHUP is running context.
I fix it and I attach third version of patch.
Zdenek
Joachim Wieland wrote:
> Zdenek,
>
> On Wed, May 31, 2006 at 06:13:04PM +0200, Zdenek Kotala wrote:
>
>> Joachim, could you explain me second point? I cannot determine
>> described problem. By my opinion my patch does not change this behavior.
>>
>
> I guess what I saw was another phenomenon:
>
> I do the following:
>
> - vi postgresql.conf => "allow_system_table_mods = true"
> - start postmaster
> - vi postgresql.conf => "# allow_system_table_mods = true" (commented)
> - killall -HUP postmaster
>
> Then I get _no_ message. After another killall -HUP I do indeed get a
> message. So I don't get it just for the first time which is strange, do you
> see that as well?
>
> However the message I get is that it got reset to its default value which is
> wrong because its a PGC_POSTMASTER variable that can only be set at server
> start (set_config_option() returns true in this case as well).
>
> Consequently I expect to get it for every other signal I send (because the
> old value is still active and differs from what is in the configuration
> file).
>
>
> Joachim
>
>
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -c -r1.37 guc-file.l
*** src/backend/utils/misc/guc-file.l 7 Mar 2006 01:03:12 -0000 1.37
--- src/backend/utils/misc/guc-file.l 1 Jun 2006 14:46:56 -0000
***************
*** 112,119 ****
void
ProcessConfigFile(GucContext context)
{
! int elevel;
struct name_value_pair *item, *head, *tail;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 112,120 ----
void
ProcessConfigFile(GucContext context)
{
! int elevel, i;
struct name_value_pair *item, *head, *tail;
+ char *env;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
***************
*** 150,155 ****
--- 151,207 ----
PGC_S_FILE, false, true);
}
+ if( context == PGC_SIGHUP)
+ {
+ /* Revert all "untouched" options with reset source PGC_S_FILE to
+ * default/boot value.
+ */
+ for (i = 0; i < num_guc_variables; i++)
+ {
+ struct config_generic *gconf = guc_variables[i];
+ if ( gconf->context != PGC_INTERNAL && gconf->context != PGC_POSTMASTER &&
+ !( gconf->context == PGC_BACKEND && IsUnderPostmaster) &&
+ (gconf->reset_source == PGC_S_FILE) && ( (gconf->status & GUC_JUST_RELOAD) == 0) )
+ {
+ if( set_config_option(gconf->name, NULL, context,
+ PGC_S_FILE, false, true) )
+ {
+ GucStack *stack;
+ /* set correctly source */
+ gconf->reset_source = PGC_S_DEFAULT;
+ for (stack = gconf->stack; stack; stack = stack->prev)
+ {
+ if (stack->source == PGC_S_FILE)
+ {
+ stack->source = PGC_S_DEFAULT;
+ }
+ }
+
+ ereport(elevel,
+ (errcode(ERRCODE_SUCCESSFUL_COMPLETION),
+ errmsg("Configuration option %s has been revert to the boot value.", gconf->name)));
+ }
+ }
+ gconf->status &= ~GUC_JUST_RELOAD;
+ }
+
+ /* Revert to environment variable. PGPORT is ignored, because it cannot be
+ * set in running state.
+ */
+ env = getenv("PGDATESTYLE");
+ if (env != NULL)
+ {
+ set_config_option("datestyle", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+
+ env = getenv("PGCLIENTENCODING");
+ if (env != NULL)
+ {
+ set_config_option("client_encoding", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+ }
cleanup_list:
free_name_value_list(head);
}
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.319
diff -c -r1.319 guc.c
*** src/backend/utils/misc/guc.c 11 May 2006 19:15:35 -0000 1.319
--- src/backend/utils/misc/guc.c 1 Jun 2006 14:46:56 -0000
***************
*** 2648,2686 ****
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_STRING:
--- 2648,2686 ----
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_STRING:
***************
*** 3133,3139 ****
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status;
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
--- 3133,3139 ----
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status & (~GUC_JUST_RELOAD);
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
***************
*** 3647,3658 ****
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
! if (changeVal && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
name)));
!
return true;
}
if (context != PGC_POSTMASTER)
--- 3647,3658 ----
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
{
! if (changeVal && value != NULL && !is_newvalue_equal(record, value))
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
name)));
! record->status |= GUC_JUST_RELOAD;
return true;
}
if (context != PGC_POSTMASTER)
***************
*** 3693,3699 ****
--- 3693,3702 ----
* backend start.
*/
if (IsUnderPostmaster)
+ {
+ record->status |= GUC_JUST_RELOAD;
return true;
+ }
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
***************
*** 3723,3729 ****
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
/*
* Ignore attempted set if overridden by previously processed setting.
--- 3726,3732 ----
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
/*
* Ignore attempted set if overridden by previously processed setting.
***************
*** 3766,3773 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3769,3787 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3850,3857 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3864,3882 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3934,3941 ****
}
else
{
! newval = conf->reset_val;
! source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3959,3977 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 4009,4014 ****
--- 4045,4064 ----
if (conf->gen.flags & GUC_IS_NAME)
truncate_identifier(newval, strlen(newval), true);
}
+ else if (source == PGC_S_FILE)
+ {
+ /* Revert value to default when item is removed from config file. */
+ if ( conf->boot_val != NULL )
+ {
+ newval = guc_strdup(elevel, conf->boot_val);
+ if (newval == NULL)
+ return false;
+ }
+ else
+ {
+ return false;
+ }
+ }
else if (conf->reset_val)
{
/*
***************
*** 4117,4122 ****
--- 4167,4175 ----
if (changeVal && (record->flags & GUC_REPORT))
ReportGUCOption(record);
+ if(changeVal && record->source == PGC_S_FILE && value != NULL)
+ record->status |= GUC_JUST_RELOAD;
+
return true;
}
***************
*** 5111,5116 ****
--- 5164,5174 ----
static bool
is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ if( !newvalue )
+ {
+ return false;
+ }
+
switch (record->vartype)
{
case PGC_BOOL:
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.22
diff -c -r1.22 guc_tables.h
*** src/include/utils/guc_tables.h 5 Mar 2006 15:59:07 -0000 1.22
--- src/include/utils/guc_tables.h 1 Jun 2006 14:46:57 -0000
***************
*** 132,138 ****
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
!
/* GUC records for specific variable types */
--- 132,138 ----
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
! #define GUC_JUST_RELOAD 0x0008 /* value is just reload from configuration file */
/* GUC records for specific variable types */
***************
*** 142,152 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool reset_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
};
struct config_int
--- 142,153 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool boot_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
+ bool reset_val;
};
struct config_int
***************
*** 155,167 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int reset_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
};
struct config_real
--- 156,169 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int boot_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
+ int reset_val;
};
struct config_real
***************
*** 170,182 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double reset_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
};
struct config_string
--- 172,186 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double boot_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
+ double reset_val;
+
};
struct config_string
Zdenek, On Thu, Jun 01, 2006 at 04:56:10PM +0200, Zdenek Kotala wrote: > Thanks, You have right. It is problem with silent skip some option in > set_config_option function when PGC_SIGHUP is running context. > I fix it and I attach third version of patch. Still it does not what I think it should do. I might have been unclear before. If you put a comment in front of a PGC_POSTMASTER variable (and if its value differs from the default) then this should be treated as if the variable got changed and it should emmit a warning "<varname> can only be changed on server start" or similar. This warning should be kept for every other SIGHUP that gets sent just like it is done already when you change the value (but do not comment the variable). I still have the problem that the first signal I send does not trigger any message (i get the "SIGHUP received", but nothing about the variable changes) but I haven't looked into it yet. Joachim
Joachim Wieland wrote:
>
> Still it does not what I think it should do. I might have been unclear
> before. If you put a comment in front of a PGC_POSTMASTER variable (and if
> its value differs from the default) then this should be treated as if the
> variable got changed and it should emmit a warning "<varname> can only be
> changed on server start" or similar. This warning should be kept for every
> other SIGHUP that gets sent just like it is done already when you change the
> value (but do not comment the variable).
>
Thanks for explanation. I overlooked this variant. When I analyzed
set_config_option I found some other bugs or strange things:
1) Try to change internal variable in the config file is silently
ignored during reconfiguration.
2) GUC_DISALLOW_IN_FILE flag is ignored during configuration file parsing.
3) If option is PGC_POSTMASTER type and value is not syntax valid, It
only generates warning message that value cannot be change and file
parsing continue.
Could some one validates my findings?
I think that set_config_options is too huge and very overloaded. By my
opinion divide to small functions is necessary to fix this behavior and
its increase maintainability.
Zdenek
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> 2) GUC_DISALLOW_IN_FILE flag is ignored during configuration file parsing.
Yeah, that's not enforced at the moment, it's just documentation.
Most of the variables that are marked that way have other defenses
against being changed from the file, so I'm not sure that it's real
important to have a specific check for it.
> 1) Try to change internal variable in the config file is silently
> ignored during reconfiguration.
Note that there are two separate behaviors: during postmaster startup,
errors in the config file are cause for aborting. During SIGHUP reread,
errors in the config file may NOT cause an abort. This is intentional.
The postmaster (and only the postmaster, not the N backends that are
also rereading the file) is supposed to emit a log message about any
problems, but it mustn't error out.
> I think that set_config_options is too huge and very overloaded. By my
> opinion divide to small functions is necessary to fix this behavior and
> its increase maintainability.
Feel free to propose a suitable refactoring.
regards, tom lane
There is last version of patch with following changes/improvements:
1) I divide set_config_option to more smaller functions without backside
effect.
2) Behavior of reconfiguration is "same" in SIG_HUP context (exclude
elevel). All errors are reported and full validity of file is checked too.
Zdenek
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -c -r1.37 guc-file.l
*** src/backend/utils/misc/guc-file.l 7 Mar 2006 01:03:12 -0000 1.37
--- src/backend/utils/misc/guc-file.l 13 Jul 2006 21:55:28 -0000
***************
*** 112,119 ****
void
ProcessConfigFile(GucContext context)
{
! int elevel;
struct name_value_pair *item, *head, *tail;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 112,120 ----
void
ProcessConfigFile(GucContext context)
{
! int elevel, i;
struct name_value_pair *item, *head, *tail;
+ char *env;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
***************
*** 137,146 ****
/* Check if all options are valid */
for (item = head; item; item = item->next)
! {
! if (!set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, false))
goto cleanup_list;
}
/* If we got here all the options checked out okay, so apply them. */
--- 138,173 ----
/* Check if all options are valid */
for (item = head; item; item = item->next)
! {
! bool isEqual, isContextOk;
!
! if (!verify_config_option(item->name, item->value, context,
! PGC_S_FILE, &isEqual, &isContextOk))
! {
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("configuration file is invalid")));
goto cleanup_list;
+ }
+
+ if( isContextOk == false )
+ {
+ if( context == PGC_SIGHUP )
+ {
+ if ( isEqual == false )
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
+ item->name)));
+ }
+ }
+ else
+ {
+ // if it is boot phase loading context must be valid for all configuration item.
+ goto cleanup_list;
+ }
+ }
}
/* If we got here all the options checked out okay, so apply them. */
***************
*** 150,155 ****
--- 177,233 ----
PGC_S_FILE, false, true);
}
+ if( context == PGC_SIGHUP)
+ {
+ /* Revert all "untouched" options with reset source PGC_S_FILE to
+ * default/boot value.
+ */
+ for (i = 0; i < num_guc_variables; i++)
+ {
+ struct config_generic *gconf = guc_variables[i];
+ if ( gconf->context != PGC_INTERNAL && gconf->context != PGC_POSTMASTER &&
+ !( gconf->context == PGC_BACKEND && IsUnderPostmaster) &&
+ (gconf->reset_source == PGC_S_FILE) && ( (gconf->status & GUC_JUST_RELOAD) == 0) )
+ {
+ if( set_config_option(gconf->name, NULL, context,
+ PGC_S_FILE, false, true) )
+ {
+ GucStack *stack;
+ /* set correctly source */
+ gconf->reset_source = PGC_S_DEFAULT;
+ for (stack = gconf->stack; stack; stack = stack->prev)
+ {
+ if (stack->source == PGC_S_FILE)
+ {
+ stack->source = PGC_S_DEFAULT;
+ }
+ }
+
+ ereport(elevel,
+ (errcode(ERRCODE_SUCCESSFUL_COMPLETION),
+ errmsg("configuration option %s has been revert to the boot value", gconf->name)));
+ }
+ }
+ gconf->status &= ~GUC_JUST_RELOAD;
+ }
+
+ /* Revert to environment variable. PGPORT is ignored, because it cannot be
+ * set in running state.
+ */
+ env = getenv("PGDATESTYLE");
+ if (env != NULL)
+ {
+ set_config_option("datestyle", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+
+ env = getenv("PGCLIENTENCODING");
+ if (env != NULL)
+ {
+ set_config_option("client_encoding", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+ }
cleanup_list:
free_name_value_list(head);
}
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.319
diff -c -r1.319 guc.c
*** src/backend/utils/misc/guc.c 11 May 2006 19:15:35 -0000 1.319
--- src/backend/utils/misc/guc.c 13 Jul 2006 21:55:28 -0000
***************
*** 2648,2686 ****
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_STRING:
--- 2648,2686 ----
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_STRING:
***************
*** 3133,3139 ****
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status;
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
--- 3133,3139 ----
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status & (~GUC_JUST_RELOAD);
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
***************
*** 3571,3630 ****
return result;
}
-
/*
! * Sets option `name' to given value. The value should be a string
! * which is going to be parsed and converted to the appropriate data
! * type. The context and source parameters indicate in which context this
! * function is being called so it can apply the access restrictions
! * properly.
! *
! * If value is NULL, set the option to its default value. If the
! * parameter changeVal is false then don't really set the option but do all
! * the checks to see if it would work.
! *
! * If there is an error (non-existing option, invalid value) then an
! * ereport(ERROR) is thrown *unless* this is called in a context where we
! * don't want to ereport (currently, startup or SIGHUP config file reread).
! * In that case we write a suitable error message via ereport(DEBUG) and
! * return false. This is working around the deficiencies in the ereport
! * mechanism, so don't blame me. In all other cases, the function
! * returns true, including cases where the input is valid but we chose
! * not to apply it because of context or source-priority considerations.
! *
! * See also SetConfigOption for an external interface.
*/
! bool
! set_config_option(const char *name, const char *value,
! GucContext context, GucSource source,
! bool isLocal, bool changeVal)
{
- struct config_generic *record;
- int elevel;
- bool makeDefault;
! if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
{
! /*
! * To avoid cluttering the log, only the postmaster bleats loudly
! * about problems with the config file.
! */
! elevel = IsUnderPostmaster ? DEBUG2 : LOG;
! }
! else if (source == PGC_S_DATABASE || source == PGC_S_USER)
! elevel = INFO;
! else
! elevel = ERROR;
! record = find_option(name, elevel);
! if (record == NULL)
! {
! ereport(elevel,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("unrecognized configuration parameter \"%s\"", name)));
! return false;
}
/*
* Check if the option can be set at this time. See guc.h for the precise
* rules. Note that we don't want to throw errors if we're in the SIGHUP
--- 3571,3852 ----
return result;
}
/*
! * Try to parse value. Determine what is type and call related
! * parsing function or if newval is equal to NULL, reset value
! * to default or bootval. If the value parsed okay return true,
! * else false.
*/
! static bool
! parse_value(int elevel, const struct config_generic *record,
! const char *value, GucSource *source, bool changeVal,
! union config_var_value *retval)
{
! Assert( !(changeVal && retval==NULL) );
! /*
! * Evaluate value and set variable.
! */
! switch (record->vartype)
{
! case PGC_BOOL:
! {
! struct config_bool *conf = (struct config_bool *) record;
! bool newval;
! if (value)
! {
! if (!parse_bool(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a Boolean value",
! record->name)));
! return false;
! }
! }
! else
! {
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! record->name, (int) newval)));
! return false;
! }
! if( retval != NULL )
! retval->boolval = newval;
! break;
! }
!
! case PGC_INT:
! {
! struct config_int *conf = (struct config_int *) record;
! int newval;
!
! if (value)
! {
! if (!parse_int(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires an integer value",
! record->name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
! newval, record->name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! record->name, newval)));
! return false;
! }
! if( retval != NULL )
! retval->intval = newval;
! break;
! }
!
! case PGC_REAL:
! {
! struct config_real *conf = (struct config_real *) record;
! double newval;
!
! if (value)
! {
! if (!parse_real(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a numeric value",
! record->name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
! newval, record->name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %g",
! record->name, newval)));
! return false;
! }
! if( retval != NULL )
! retval->realval = newval;
! break;
! }
!
! case PGC_STRING:
! {
! struct config_string *conf = (struct config_string *) record;
! char *newval;
!
! if (value)
! {
! newval = guc_strdup(elevel, value);
! if (newval == NULL)
! return false;
! /*
! * The only sort of "parsing" check we need to do is
! * apply truncation if GUC_IS_NAME.
! */
! if (conf->gen.flags & GUC_IS_NAME)
! truncate_identifier(newval, strlen(newval), true);
! }
! else if (*source == PGC_S_FILE)
! {
! /* Revert value to default when item is removed from config file. */
! if ( conf->boot_val != NULL )
! {
! newval = guc_strdup(elevel, conf->boot_val);
! if (newval == NULL)
! return false;
! }
! else
! {
! return false;
! }
! }
! else if (conf->reset_val)
! {
! /*
! * We could possibly avoid strdup here, but easier to make
! * this case work the same as the normal assignment case.
! */
! newval = guc_strdup(elevel, conf->reset_val);
! if (newval == NULL)
! return false;
! *source = conf->gen.reset_source;
! }
! else
! {
! /* Nothing to reset to, as yet; so do nothing */
! break;
! }
!
! if (conf->assign_hook)
! {
! const char *hookresult;
!
! /*
! * If the hook ereports, we have to make sure we free
! * newval, else it will be a permanent memory leak.
! */
! hookresult = call_string_assign_hook(conf->assign_hook,
! newval,
! changeVal,
! *source);
! if (hookresult == NULL)
! {
! free(newval);
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": \"%s\"",
! record->name, value ? value : "")));
! return false;
! }
! else if (hookresult != newval)
! {
! free(newval);
!
! /*
! * Having to cast away const here is annoying, but the
! * alternative is to declare assign_hooks as returning
! * char*, which would mean they'd have to cast away
! * const, or as both taking and returning char*, which
! * doesn't seem attractive either --- we don't want
! * them to scribble on the passed str.
! */
! newval = (char *) hookresult;
! }
! }
!
! if ( !changeVal )
! free(newval);
! if( retval != NULL )
! retval->stringval= newval;
! break;
! }
}
+ return true;
+ }
+ /*
+ *
+ */
+ bool
+ checkContext(int elevel, struct config_generic *record, GucContext context)
+ {
/*
* Check if the option can be set at this time. See guc.h for the precise
* rules. Note that we don't want to throw errors if we're in the SIGHUP
***************
*** 3633,3666 ****
switch (record->context)
{
case PGC_INTERNAL:
- if (context == PGC_SIGHUP)
- return true;
if (context != PGC_INTERNAL)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed",
! name)));
return false;
}
break;
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
! {
! if (changeVal && !is_newvalue_equal(record, value))
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
! name)));
- return true;
- }
if (context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start",
! name)));
return false;
}
break;
--- 3855,3879 ----
switch (record->context)
{
case PGC_INTERNAL:
if (context != PGC_INTERNAL)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed",
! record->name)));
return false;
}
break;
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
! return false;
if (context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start",
! record->name)));
return false;
}
break;
***************
*** 3670,3676 ****
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed now",
! name)));
return false;
}
--- 3883,3889 ----
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed now",
! record->name)));
return false;
}
***************
*** 3693,3706 ****
* backend start.
*/
if (IsUnderPostmaster)
! return true;
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be set after connection start",
! name)));
return false;
}
break;
--- 3906,3921 ----
* backend start.
*/
if (IsUnderPostmaster)
! {
! return false;
! }
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be set after connection start",
! record->name)));
return false;
}
break;
***************
*** 3710,3716 ****
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set parameter \"%s\"",
! name)));
return false;
}
break;
--- 3925,3931 ----
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set parameter \"%s\"",
! record->name)));
return false;
}
break;
***************
*** 3718,3729 ****
/* always okay */
break;
}
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
/*
* Ignore attempted set if overridden by previously processed setting.
--- 3933,4052 ----
/* always okay */
break;
}
+ return true;
+ }
+ /*
+ * Get error level for different sources and context.
+ */
+ int
+ get_elevel(GucContext context, GucSource source)
+ {
+ int elevel;
+ if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
+ {
+ /*
+ * To avoid cluttering the log, only the postmaster bleats loudly
+ * about problems with the config file.
+ */
+ elevel = IsUnderPostmaster ? DEBUG2 : LOG;
+ }
+ else if (source == PGC_S_DATABASE || source == PGC_S_USER)
+ elevel = INFO;
+ else
+ elevel = ERROR;
+
+ return elevel;
+ }
+
+ /*
+ * Verify if option exists and value is valid.
+ * It is primary used for validation of items in configuration file.
+ */
+ bool
+ verify_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool *isNewEqual, bool *isContextOK)
+ {
+ union config_var_value newval;
+ int elevel;
+ struct config_generic *record;
+
+ elevel = get_elevel(context, source);
+
+ record = find_option(name, elevel);
+ if (record == NULL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ return false;
+ }
+
+ if( parse_value(elevel, record, value, &source, false, &newval) )
+ {
+ if( isNewEqual != NULL) *isNewEqual = is_newvalue_equal(record, value);
+ if( isContextOK != NULL) *isContextOK = checkContext(elevel, record, context);
+ }
+ else
+ return false;
+
+ return true;
+ }
+
+
+ /*
+ * Sets option `name' to given value. The value should be a string
+ * which is going to be parsed and converted to the appropriate data
+ * type. The context and source parameters indicate in which context this
+ * function is being called so it can apply the access restrictions
+ * properly.
+ *
+ * If value is NULL, set the option to its default value. If the
+ * parameter changeVal is false then don't really set the option but do all
+ * the checks to see if it would work.
+ *
+ * If there is an error (non-existing option, invalid value) then an
+ * ereport(ERROR) is thrown *unless* this is called in a context where we
+ * don't want to ereport (currently, startup or SIGHUP config file reread).
+ * In that case we write a suitable error message via ereport(DEBUG) and
+ * return false. This is working around the deficiencies in the ereport
+ * mechanism, so don't blame me. In all other cases, the function
+ * returns true, including cases where the input is valid but we chose
+ * not to apply it because of context or source-priority considerations.
+ *
+ * See also SetConfigOption for an external interface.
+ */
+ bool
+ set_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool isLocal, bool changeVal)
+ {
+ struct config_generic *record;
+ int elevel;
+ bool makeDefault;
+
+ elevel = get_elevel(context, source);
+
+ record = find_option(name, elevel);
+ if (record == NULL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ return false;
+ }
+
+ /* Check if change is allowed in the running context. */
+ if( !checkContext )
+ {
+ return false;
+ }
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
/*
* Ignore attempted set if overridden by previously processed setting.
***************
*** 3752,3784 ****
{
struct config_bool *conf = (struct config_bool *) record;
bool newval;
!
! if (value)
! {
! if (!parse_bool(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a Boolean value",
! name)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! name, (int) newval)));
! return false;
! }
if (changeVal || makeDefault)
{
--- 4075,4083 ----
{
struct config_bool *conf = (struct config_bool *) record;
bool newval;
!
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 3829,3868 ****
struct config_int *conf = (struct config_int *) record;
int newval;
! if (value)
! {
! if (!parse_int(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires an integer value",
! name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
! newval, name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! name, newval)));
! return false;
! }
if (changeVal || makeDefault)
{
--- 4128,4135 ----
struct config_int *conf = (struct config_int *) record;
int newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 3913,3952 ****
struct config_real *conf = (struct config_real *) record;
double newval;
! if (value)
! {
! if (!parse_real(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a numeric value",
! name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
! newval, name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %g",
! name, newval)));
! return false;
! }
if (changeVal || makeDefault)
{
--- 4180,4187 ----
struct config_real *conf = (struct config_real *) record;
double newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 3997,4067 ****
struct config_string *conf = (struct config_string *) record;
char *newval;
! if (value)
! {
! newval = guc_strdup(elevel, value);
! if (newval == NULL)
! return false;
! /*
! * The only sort of "parsing" check we need to do is
! * apply truncation if GUC_IS_NAME.
! */
! if (conf->gen.flags & GUC_IS_NAME)
! truncate_identifier(newval, strlen(newval), true);
! }
! else if (conf->reset_val)
! {
! /*
! * We could possibly avoid strdup here, but easier to make
! * this case work the same as the normal assignment case.
! */
! newval = guc_strdup(elevel, conf->reset_val);
! if (newval == NULL)
! return false;
! source = conf->gen.reset_source;
! }
! else
! {
! /* Nothing to reset to, as yet; so do nothing */
! break;
! }
!
! if (conf->assign_hook)
! {
! const char *hookresult;
!
! /*
! * If the hook ereports, we have to make sure we free
! * newval, else it will be a permanent memory leak.
! */
! hookresult = call_string_assign_hook(conf->assign_hook,
! newval,
! changeVal,
! source);
! if (hookresult == NULL)
! {
! free(newval);
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": \"%s\"",
! name, value ? value : "")));
! return false;
! }
! else if (hookresult != newval)
! {
! free(newval);
!
! /*
! * Having to cast away const here is annoying, but the
! * alternative is to declare assign_hooks as returning
! * char*, which would mean they'd have to cast away
! * const, or as both taking and returning char*, which
! * doesn't seem attractive either --- we don't want
! * them to scribble on the passed str.
! */
! newval = (char *) hookresult;
! }
! }
if (changeVal || makeDefault)
{
--- 4232,4239 ----
struct config_string *conf = (struct config_string *) record;
char *newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 4109,4115 ****
}
}
else
! free(newval);
break;
}
}
--- 4281,4288 ----
}
}
else
! if( newval != NULL )
! free(newval);
break;
}
}
***************
*** 4117,4122 ****
--- 4290,4298 ----
if (changeVal && (record->flags & GUC_REPORT))
ReportGUCOption(record);
+ if(changeVal && record->source == PGC_S_FILE && value != NULL)
+ record->status |= GUC_JUST_RELOAD;
+
return true;
}
***************
*** 5111,5116 ****
--- 5287,5297 ----
static bool
is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ if( !newvalue )
+ {
+ return false;
+ }
+
switch (record->vartype)
{
case PGC_BOOL:
Index: src/include/utils/guc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
retrieving revision 1.68
diff -c -r1.68 guc.h
*** src/include/utils/guc.h 11 May 2006 19:15:35 -0000 1.68
--- src/include/utils/guc.h 13 Jul 2006 21:55:29 -0000
***************
*** 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 ----
***************
*** 195,200 ****
--- 194,202 ----
extern bool set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
bool isLocal, bool changeVal);
+ extern bool verify_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool *isNewEqual, bool *isContextOK);
extern char *GetConfigOptionByName(const char *name, const char **varname);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.22
diff -c -r1.22 guc_tables.h
*** src/include/utils/guc_tables.h 5 Mar 2006 15:59:07 -0000 1.22
--- src/include/utils/guc_tables.h 13 Jul 2006 21:55:29 -0000
***************
*** 132,138 ****
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
!
/* GUC records for specific variable types */
--- 132,138 ----
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
! #define GUC_JUST_RELOAD 0x0008 /* value is just reload from configuration file */
/* GUC records for specific variable types */
***************
*** 142,152 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool reset_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
};
struct config_int
--- 142,153 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool boot_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
+ bool reset_val;
};
struct config_int
***************
*** 155,167 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int reset_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
};
struct config_real
--- 156,169 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int boot_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
+ int reset_val;
};
struct config_real
***************
*** 170,182 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double reset_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
};
struct config_string
--- 172,186 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double boot_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
+ double reset_val;
+
};
struct config_string
Zdenek,
On Fri, Jul 14, 2006 at 12:17:55AM +0200, Zdenek Kotala wrote:
> There is last version of patch with following changes/improvements:
> 1) I divide set_config_option to more smaller functions without backside
> effect.
I did not check the changes you have done to set_config_option and the like
but tested the commenting / uncommenting / changing of guc variables and the
behavior and log output. The general idea (at least my idea) is that
whenever a SIGHUP is received and there is some difference between the
config file and the active value that the server is using, a notice message
is written to the log. That way, at every moment you can see if the active
values coincide with the configuration file by sending a SIGHUP and if there
are no such messages the admin can stop and restart the server and be sure
that the settings will be the same after a restart.
While testing, I specified a bunch of test cases that I attach below.
I also renamed the GUC_JUST_RELOAD to GUC_IN_CONFFILE because I did not
really understand what GUC_JUST_RELOAD should mean. GUC_IN_CONFFILE means
that the variable does show up in the configuration file and is active
there, i.e. is not commented.
Please check my changes, I'm pretty sure it can be cleaned up further.
Joachim
Test cases for "guc falls back to default":
guc_context <= PGC_POSTMASTER (shared_buffers is an example, Default: 1000)
Commented value gets un-commented (value != default)
=> message every time a sighup is received
Example:
#shared_buffers = 3301
START
shared_buffers = 3301
HUP
Output:
LOG: parameter "shared_buffers" cannot be changed after server start;
configuration file change ignored
Value gets changed (to != initial).
=> message every time a sighup is received
Example:
shared_buffers = 3301
START
shared_buffers = 3302
HUP
Output:
LOG: parameter "max_prepared_transactions" cannot be changed after
server start; configuration file change ignored
Value gets commented (initial != default).
=> message every time a sighup is received
Example:
shared_buffers = 3301
START
#shared_buffers = 3301
HUP
Output:
LOG: parameter "max_prepared_transactions" cannot be changed
(commented) after server start; configuration file change ignored
Commented value (not applied) gets changed back to initial setting:
=> no more messages after SIGHUP
Example:
shared_buffers = 3301
START
#shared_buffers = 3301
HUP (value does not get applied)
shared_buffers = 3301
HUP
Output:
None
Commented value (not applied) gets changed to != initial:
=> message every time a SIGHUP is received
Example:
shared_buffers = 3301
START
#shared_buffers = 3301
HUP
shared_buffers = 3302
HUP
Output:
LOG: parameter "shared_buffers" cannot be changed after server start;
configuration file change ignored
guc_context <= PGC_SIGHUP set (fsync is an example, Default: true)
Value (== default) gets commented
=> nothing happens
Example:
fsync = true
START
#fsync = true
HUP
Output:
None
Value (!= default) gets commented
=> falls back to default on first HUP that is received)
Example:
fsync = false
START
fsync = true
HUP
(subsequent HUPs do not show output anymore)
Output:
LOG: configuration option fsync falls back to default value
Commented value gets un-commented (value != default)
Example:
#fsync = false
START
fsync = false
HUP
Output:
None
Commented value gets un-commented (value == default)
Example:
#fsync = true
START
fsync = true
HUP
Output:
None
Attachment
Joachim Wieland <joe@mcknight.de> writes:
> I did not check the changes you have done to set_config_option and the like
> but tested the commenting / uncommenting / changing of guc variables and the
> behavior and log output. The general idea (at least my idea) is that
> whenever a SIGHUP is received and there is some difference between the
> config file and the active value that the server is using, a notice message
> is written to the log.
Notice message? Where did that come from? The behavior I thought
people were after was just that variables previously defined by the file
would revert to reset values if not any longer defined by the file.
From a reviewer's point of view, it'd be nice if the patch did not
contain so many useless changes of whitespace.
regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Joachim Wieland <joe@mcknight.de> writes:
> > I did not check the changes you have done to set_config_option and the like
> > but tested the commenting / uncommenting / changing of guc variables and the
> > behavior and log output. The general idea (at least my idea) is that
> > whenever a SIGHUP is received and there is some difference between the
> > config file and the active value that the server is using, a notice message
> > is written to the log.
>
> Notice message? Where did that come from? The behavior I thought
> people were after was just that variables previously defined by the file
> would revert to reset values if not any longer defined by the file.
There's two issues here, I believe. There's the
'revert-to-reset-values' issue for things which can be changed with a
reload and then there's also the 'notice-message-if-unable-to-change'
a given variable without a reset.
On reload a variable is changed:
#1: That variable can be changed by a reload.
If the variable has been removed/commented-out then it is reverted
to the reset-value. Otherwise, the new value is used.
#2: That variable can *not* be changed by a reload.
Notice-level message is sent to the log notifying the admin that the
change requested could not be performed. This change could be
either a revert to reset-value if it was removed/commented-out or an
explicit change request to a different value.
Personally, I'm very interested in having both. I'm about 90% sure both
were discussed previously on hackers and that the general consensus was
that both were good. It's possible the second point wasn't noticed by
everyone involved though. Of course, I might be misunderstanding what
Joachim was referring to also.
Thanks,
Stephen
Am Montag, 24. Juli 2006 16:55 schrieb Stephen Frost: > #2: That variable can *not* be changed by a reload. > Notice-level message is sent to the log notifying the admin that the > change requested could not be performed. This already happens. -- Peter Eisentraut http://developer.postgresql.org/~petere/
On Mon, Jul 24, 2006 at 07:09:17PM +0200, Peter Eisentraut wrote: > Am Montag, 24. Juli 2006 16:55 schrieb Stephen Frost: > > #2: That variable can *not* be changed by a reload. > > Notice-level message is sent to the log notifying the admin that the > > change requested could not be performed. > This already happens. Not if the option gets commented/deleted, i.e.: shared_buffers = 8000 START #shared_buffers = 8000 HUP This does not issue a message at the moment. Joachim
On Mon, Jul 24, 2006 at 10:55:47AM -0400, Stephen Frost wrote: > #2: That variable can *not* be changed by a reload. > Notice-level message is sent to the log notifying the admin that the > change requested could not be performed. This change could be > either a revert to reset-value if it was removed/commented-out or an > explicit change request to a different value. Right. And what I am voting for is to not only issue such a message once but every time a SIGHUP is received as long as the actively-used value differs from the value in the configuration file. One of the reasons for having this fall-back-to-default-value stuff is to make sure that an admin can restart a server and be sure that it will behave in the same way as when it was shut down. Moreover it's just clearer to send the notice message every time a SIGHUP is received since every reload is the admin's request to apply all of the values in the configuration file independently of what has happened in the past. Joachim
Joachim Wieland wrote: > On Mon, Jul 24, 2006 at 07:09:17PM +0200, Peter Eisentraut wrote: > > Am Montag, 24. Juli 2006 16:55 schrieb Stephen Frost: > > > #2: That variable can *not* be changed by a reload. > > > Notice-level message is sent to the log notifying the admin > > > that the change requested could not be performed. > > > > This already happens. > > Not if the option gets commented/deleted, i.e.: > > shared_buffers = 8000 > START > #shared_buffers = 8000 > HUP > > This does not issue a message at the moment. Because at the moment, the above does not change the value of shared_buffers. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Joachim Wieland wrote:
> Zdenek,
>
> The general idea (at least my idea) is that
> whenever a SIGHUP is received and there is some difference between the
> config file and the active value that the server is using, a notice message
> is written to the log. That way, at every moment you can see if the active
> values coincide with the configuration file by sending a SIGHUP and if there
> are no such messages the admin can stop and restart the server and be sure
> that the settings will be the same after a restart.
>
> While testing, I specified a bunch of test cases that I attach below.
It would be nice to have this like regression test. I'm still thinking
how to realize it. There is problem how to change postgresql.conf
without backside effect on other test and for all platform.
> I also renamed the GUC_JUST_RELOAD to GUC_IN_CONFFILE because I did not
> really understand what GUC_JUST_RELOAD should mean. GUC_IN_CONFFILE means
> that the variable does show up in the configuration file and is active
> there, i.e. is not commented.
Yes, you have right, JUST_RELOAD is not much clear. I look that you
little bit changed function of this flag to. I think it is better and
more clear. However, I have some doubt that in the some special case -
SET/RESET usage in transaction - should generate some wrong output. I'm
working on some deep analyze and test case.
>
> Please check my changes, I'm pretty sure it can be cleaned up further.
Thanks for your improvement.
Zdenek
Joachim,
I checked your improvement and fix some problem with following scenario:
shared_buffers = 3301
START
share_buffers = 333.39
HUP
share_buffers requires integer value. Skip configuration file
#share_buffers = 3301
HUP
.... silent - no message
I performed some cleanup in my code as well. I reduced some conditions,
which cannot occur and fixed context validation in the
set_config_options function. I hope that It is final version of our patch.
Thanks for cooperation
Zdenek
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -c -r1.37 guc-file.l
*** src/backend/utils/misc/guc-file.l 7 Mar 2006 01:03:12 -0000 1.37
--- src/backend/utils/misc/guc-file.l 27 Jul 2006 14:07:46 -0000
***************
*** 50,56 ****
static bool ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, 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);
--- 50,57 ----
static bool ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, int elevel,
struct name_value_pair **head_p,
! struct name_value_pair **tail_p,
! int *varcount);
static void free_name_value_list(struct name_value_pair * list);
static char *GUC_scanstr(const char *s);
***************
*** 112,119 ****
void
ProcessConfigFile(GucContext context)
{
! int elevel;
struct name_value_pair *item, *head, *tail;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 113,123 ----
void
ProcessConfigFile(GucContext context)
{
! int elevel, i;
struct name_value_pair *item, *head, *tail;
+ char *env;
+ bool *apply_list = NULL;
+ int varcount = 0;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
***************
*** 132,156 ****
if (!ParseConfigFile(ConfigFileName, NULL,
0, context, elevel,
! &head, &tail))
goto cleanup_list;
/* Check if all options are valid */
! for (item = head; item; item = item->next)
{
! if (!set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, false))
goto cleanup_list;
}
/* If we got here all the options checked out okay, so apply them. */
! for (item = head; item; item = item->next)
{
! set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, true);
}
! cleanup_list:
free_name_value_list(head);
}
--- 136,243 ----
if (!ParseConfigFile(ConfigFileName, NULL,
0, context, elevel,
! &head, &tail, &varcount))
goto cleanup_list;
+ /* Can we allocate memory here, what about leaving here prematurely? */
+ apply_list = (bool *) palloc(sizeof(bool) * varcount);
+
/* Check if all options are valid */
! for (item = head, i = 0; item; item = item->next, i++)
{
! bool isEqual, isContextOk;
!
! if (!verify_config_option(item->name, item->value, context,
! PGC_S_FILE, &isEqual, &isContextOk))
! {
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("configuration file is invalid")));
goto cleanup_list;
+ }
+
+ if( isContextOk == false )
+ {
+ apply_list[i] = false;
+ if( context == PGC_SIGHUP )
+ {
+ if ( isEqual == false )
+ ereport(elevel,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
+ item->name)));
+ }
+ else
+ /* if it is boot phase, context must be valid for all
+ * configuration item. */
+ goto cleanup_list;
+ }
+ else
+ apply_list[i] = true;
}
/* If we got here all the options checked out okay, so apply them. */
! for (item = head, i = 0; item; item = item->next, i++)
! if (apply_list[i])
! set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, true);
!
! if( context == PGC_SIGHUP)
{
! /*
! * Revert all "untouched" options with reset source PGC_S_FILE to
! * default/boot value.
! */
! for (i = 0; i < num_guc_variables; i++)
! {
! struct config_generic *gconf = guc_variables[i];
! if ( gconf->reset_source == PGC_S_FILE &&
! !(gconf->status & GUC_IN_CONFFILE) )
! {
! if ( gconf->context == PGC_BACKEND && IsUnderPostmaster)
! ; /* Be silent. Does any body want message from each session? */
! else if (gconf->context == PGC_POSTMASTER)
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("parameter \"%s\" cannot be changed (commented) after server start; configuration file
changeignored",
! gconf->name)));
! else if(set_config_option(gconf->name,
! NULL, context,
! PGC_S_FILE,
! false, true))
! {
! GucStack *stack;
! /* set correctly source */
! gconf->reset_source = PGC_S_DEFAULT;
! for (stack = gconf->stack; stack; stack = stack->prev)
! if (stack->source == PGC_S_FILE)
! stack->source = PGC_S_DEFAULT;
!
! ereport(elevel,
! (errcode(ERRCODE_SUCCESSFUL_COMPLETION),
! errmsg("configuration option %s falls back to default value", gconf->name)));
! }
! }
! gconf->status &= ~GUC_IN_CONFFILE;
! }
!
! /* Revert to environment variable. PGPORT is ignored, because it cannot be
! * set in running state.
! */
! env = getenv("PGDATESTYLE");
! if (env != NULL)
! set_config_option("datestyle", env, context,
! PGC_S_ENV_VAR, false, true);
!
! env = getenv("PGCLIENTENCODING");
! if (env != NULL)
! set_config_option("client_encoding", env, context,
! PGC_S_ENV_VAR, false, true);
}
! cleanup_list:
! if (apply_list)
! pfree(apply_list);
free_name_value_list(head);
}
***************
*** 187,199 ****
ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, 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
--- 274,287 ----
ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, int elevel,
struct name_value_pair **head_p,
! struct name_value_pair **tail_p,
! int *varcount)
{
! 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
***************
*** 261,269 ****
/* now we must have the option value */
if (token != GUC_ID &&
! token != GUC_STRING &&
! token != GUC_INTEGER &&
! token != GUC_REAL &&
token != GUC_UNQUOTED_STRING)
goto parse_error;
if (token == GUC_STRING) /* strip quotes and escapes */
--- 349,357 ----
/* now we must have the option value */
if (token != GUC_ID &&
! token != GUC_STRING &&
! token != GUC_INTEGER &&
! token != GUC_REAL &&
token != GUC_UNQUOTED_STRING)
goto parse_error;
if (token == GUC_STRING) /* strip quotes and escapes */
***************
*** 287,293 ****
if (!ParseConfigFile(opt_value, config_file,
depth + 1, context, elevel,
! head_p, tail_p))
{
pfree(opt_name);
pfree(opt_value);
--- 375,381 ----
if (!ParseConfigFile(opt_value, config_file,
depth + 1, context, elevel,
! head_p, tail_p, varcount))
{
pfree(opt_name);
pfree(opt_value);
***************
*** 331,336 ****
--- 419,425 ----
else
(*tail_p)->next = item;
*tail_p = item;
+ (*varcount)++;
}
/* break out of loop if read EOF, else loop for next line */
***************
*** 350,356 ****
else
ereport(elevel,
(errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("syntax error in file \"%s\" line %u, near token \"%s\"",
config_file, ConfigFileLineno, yytext)));
OK = false;
--- 439,445 ----
else
ereport(elevel,
(errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("syntax error in file \"%s\" line %u, near token \"%s\"",
config_file, ConfigFileLineno, yytext)));
OK = false;
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.329
diff -c -r1.329 guc.c
*** src/backend/utils/misc/guc.c 25 Jul 2006 03:51:21 -0000 1.329
--- src/backend/utils/misc/guc.c 27 Jul 2006 14:07:47 -0000
***************
*** 2667,2705 ****
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_STRING:
--- 2667,2705 ----
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_INT:
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_REAL:
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_STRING:
***************
*** 3152,3158 ****
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status;
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
--- 3152,3158 ----
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status & (~GUC_IN_CONFFILE);
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
***************
*** 3590,3649 ****
return result;
}
-
/*
! * Sets option `name' to given value. The value should be a string
! * which is going to be parsed and converted to the appropriate data
! * type. The context and source parameters indicate in which context this
! * function is being called so it can apply the access restrictions
! * properly.
! *
! * If value is NULL, set the option to its default value. If the
! * parameter changeVal is false then don't really set the option but do all
! * the checks to see if it would work.
! *
! * If there is an error (non-existing option, invalid value) then an
! * ereport(ERROR) is thrown *unless* this is called in a context where we
! * don't want to ereport (currently, startup or SIGHUP config file reread).
! * In that case we write a suitable error message via ereport(DEBUG) and
! * return false. This is working around the deficiencies in the ereport
! * mechanism, so don't blame me. In all other cases, the function
! * returns true, including cases where the input is valid but we chose
! * not to apply it because of context or source-priority considerations.
! *
! * See also SetConfigOption for an external interface.
*/
! bool
! set_config_option(const char *name, const char *value,
! GucContext context, GucSource source,
! bool isLocal, bool changeVal)
{
- struct config_generic *record;
- int elevel;
- bool makeDefault;
! if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
{
! /*
! * To avoid cluttering the log, only the postmaster bleats loudly
! * about problems with the config file.
! */
! elevel = IsUnderPostmaster ? DEBUG2 : LOG;
! }
! else if (source == PGC_S_DATABASE || source == PGC_S_USER)
! elevel = INFO;
! else
! elevel = ERROR;
! record = find_option(name, elevel);
! if (record == NULL)
! {
! ereport(elevel,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("unrecognized configuration parameter \"%s\"", name)));
! return false;
}
/*
* Check if the option can be set at this time. See guc.h for the precise
* rules. Note that we don't want to throw errors if we're in the SIGHUP
--- 3590,3871 ----
return result;
}
/*
! * Try to parse value. Determine what is type and call related
! * parsing function or if newval is equal to NULL, reset value
! * to default or bootval. If the value parsed okay return true,
! * else false.
*/
! static bool
! parse_value(int elevel, const struct config_generic *record,
! const char *value, GucSource *source, bool changeVal,
! union config_var_value *retval)
{
! Assert( !(changeVal && retval==NULL) );
! /*
! * Evaluate value and set variable.
! */
! switch (record->vartype)
{
! case PGC_BOOL:
! {
! struct config_bool *conf = (struct config_bool *) record;
! bool newval;
! if (value)
! {
! if (!parse_bool(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a Boolean value",
! record->name)));
! return false;
! }
! }
! else
! {
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! record->name, (int) newval)));
! return false;
! }
! if( retval != NULL )
! retval->boolval = newval;
! break;
! }
!
! case PGC_INT:
! {
! struct config_int *conf = (struct config_int *) record;
! int newval;
!
! if (value)
! {
! if (!parse_int(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires an integer value",
! record->name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
! newval, record->name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! record->name, newval)));
! return false;
! }
! if( retval != NULL )
! retval->intval = newval;
! break;
! }
!
! case PGC_REAL:
! {
! struct config_real *conf = (struct config_real *) record;
! double newval;
!
! if (value)
! {
! if (!parse_real(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a numeric value",
! record->name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
! newval, record->name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %g",
! record->name, newval)));
! return false;
! }
! if( retval != NULL )
! retval->realval = newval;
! break;
! }
!
! case PGC_STRING:
! {
! struct config_string *conf = (struct config_string *) record;
! char *newval;
!
! if (value)
! {
! newval = guc_strdup(elevel, value);
! if (newval == NULL)
! return false;
! /*
! * The only sort of "parsing" check we need to do is
! * apply truncation if GUC_IS_NAME.
! */
! if (conf->gen.flags & GUC_IS_NAME)
! truncate_identifier(newval, strlen(newval), true);
! }
! else if (*source == PGC_S_FILE)
! {
! /* Revert value to default when item is removed from config file. */
! if ( conf->boot_val != NULL )
! {
! newval = guc_strdup(elevel, conf->boot_val);
! if (newval == NULL)
! return false;
! }
! else
! {
! return false;
! }
! }
! else if (conf->reset_val)
! {
! /*
! * We could possibly avoid strdup here, but easier to make
! * this case work the same as the normal assignment case.
! */
! newval = guc_strdup(elevel, conf->reset_val);
! if (newval == NULL)
! return false;
! *source = conf->gen.reset_source;
! }
! else
! {
! /* Nothing to reset to, as yet; so do nothing */
! break;
! }
!
! if (conf->assign_hook)
! {
! const char *hookresult;
!
! /*
! * If the hook ereports, we have to make sure we free
! * newval, else it will be a permanent memory leak.
! */
! hookresult = call_string_assign_hook(conf->assign_hook,
! newval,
! changeVal,
! *source);
! if (hookresult == NULL)
! {
! free(newval);
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": \"%s\"",
! record->name, value ? value : "")));
! return false;
! }
! else if (hookresult != newval)
! {
! free(newval);
!
! /*
! * Having to cast away const here is annoying, but the
! * alternative is to declare assign_hooks as returning
! * char*, which would mean they'd have to cast away
! * const, or as both taking and returning char*, which
! * doesn't seem attractive either --- we don't want
! * them to scribble on the passed str.
! */
! newval = (char *) hookresult;
! }
! }
!
! if ( !changeVal )
! free(newval);
! if( retval != NULL )
! retval->stringval= newval;
! break;
! }
}
+ return true;
+ }
+ /*
+ *
+ */
+ bool
+ checkContext(int elevel, struct config_generic *record, GucContext context)
+ {
/*
* Check if the option can be set at this time. See guc.h for the precise
* rules. Note that we don't want to throw errors if we're in the SIGHUP
***************
*** 3652,3685 ****
switch (record->context)
{
case PGC_INTERNAL:
- if (context == PGC_SIGHUP)
- return true;
if (context != PGC_INTERNAL)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed",
! name)));
return false;
}
break;
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
! {
! if (changeVal && !is_newvalue_equal(record, value))
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
! name)));
- return true;
- }
if (context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start",
! name)));
return false;
}
break;
--- 3874,3898 ----
switch (record->context)
{
case PGC_INTERNAL:
if (context != PGC_INTERNAL)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed",
! record->name)));
return false;
}
break;
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
! return false;
if (context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start",
! record->name)));
return false;
}
break;
***************
*** 3689,3695 ****
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed now",
! name)));
return false;
}
--- 3902,3908 ----
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed now",
! record->name)));
return false;
}
***************
*** 3712,3725 ****
* backend start.
*/
if (IsUnderPostmaster)
! return true;
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be set after connection start",
! name)));
return false;
}
break;
--- 3925,3940 ----
* backend start.
*/
if (IsUnderPostmaster)
! {
! return false;
! }
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be set after connection start",
! record->name)));
return false;
}
break;
***************
*** 3729,3735 ****
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set parameter \"%s\"",
! name)));
return false;
}
break;
--- 3944,3950 ----
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set parameter \"%s\"",
! record->name)));
return false;
}
break;
***************
*** 3737,3748 ****
/* always okay */
break;
}
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
/*
* Ignore attempted set if overridden by previously processed setting.
--- 3952,4076 ----
/* always okay */
break;
}
+ return true;
+ }
+ /*
+ * Get error level for different sources and context.
+ */
+ int
+ get_elevel(GucContext context, GucSource source)
+ {
+ int elevel;
+ if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
+ {
+ /*
+ * To avoid cluttering the log, only the postmaster bleats loudly
+ * about problems with the config file.
+ */
+ elevel = IsUnderPostmaster ? DEBUG2 : LOG;
+ }
+ else if (source == PGC_S_DATABASE || source == PGC_S_USER)
+ elevel = INFO;
+ else
+ elevel = ERROR;
+
+ return elevel;
+ }
+
+ /*
+ * Verify if option exists and value is valid.
+ * It is primary used for validation of items in configuration file.
+ */
+ bool
+ verify_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool *isNewEqual, bool *isContextOK)
+ {
+ union config_var_value newval;
+ int elevel;
+ struct config_generic *record;
+
+ elevel = get_elevel(context, source);
+
+ record = find_option(name, elevel);
+ if (record == NULL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ return false;
+ }
+
+ if( parse_value(elevel, record, value, &source, false, &newval) )
+ {
+ /* Mark record like presented in the config file. Be carefull if
+ * you use this function for another purpose than config file
+ * verification. It causes confusion configfile parser. */
+ record->status |= GUC_IN_CONFFILE;
+
+ if( isNewEqual != NULL)
+ *isNewEqual = is_newvalue_equal(record, value);
+ if( isContextOK != NULL)
+ *isContextOK = checkContext(elevel, record, context);
+ }
+ else
+ return false;
+
+ return true;
+ }
+
+
+ /*
+ * Sets option `name' to given value. The value should be a string
+ * which is going to be parsed and converted to the appropriate data
+ * type. The context and source parameters indicate in which context this
+ * function is being called so it can apply the access restrictions
+ * properly.
+ *
+ * If value is NULL, set the option to its default value. If the
+ * parameter changeVal is false then don't really set the option but do all
+ * the checks to see if it would work.
+ *
+ * If there is an error (non-existing option, invalid value) then an
+ * ereport(ERROR) is thrown *unless* this is called in a context where we
+ * don't want to ereport (currently, startup or SIGHUP config file reread).
+ * In that case we write a suitable error message via ereport(DEBUG) and
+ * return false. This is working around the deficiencies in the ereport
+ * mechanism, so don't blame me. In all other cases, the function
+ * returns true, including cases where the input is valid but we chose
+ * not to apply it because of context or source-priority considerations.
+ *
+ * See also SetConfigOption for an external interface.
+ */
+ bool
+ set_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool isLocal, bool changeVal)
+ {
+ struct config_generic *record;
+ int elevel;
+ bool makeDefault;
+
+ elevel = get_elevel(context, source);
+
+ record = find_option(name, elevel);
+ if (record == NULL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ return false;
+ }
+
+ /* Check if change is allowed in the running context. */
+ if( !checkContext(elevel, record, context) )
+ return false;
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
/*
* Ignore attempted set if overridden by previously processed setting.
***************
*** 3771,3803 ****
{
struct config_bool *conf = (struct config_bool *) record;
bool newval;
!
! if (value)
! {
! if (!parse_bool(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a Boolean value",
! name)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! name, (int) newval)));
! return false;
! }
if (changeVal || makeDefault)
{
--- 4099,4107 ----
{
struct config_bool *conf = (struct config_bool *) record;
bool newval;
!
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 3848,3887 ****
struct config_int *conf = (struct config_int *) record;
int newval;
! if (value)
! {
! if (!parse_int(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires an integer value",
! name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
! newval, name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! name, newval)));
! return false;
! }
if (changeVal || makeDefault)
{
--- 4152,4159 ----
struct config_int *conf = (struct config_int *) record;
int newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 3932,3971 ****
struct config_real *conf = (struct config_real *) record;
double newval;
! if (value)
! {
! if (!parse_real(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a numeric value",
! name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
! newval, name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %g",
! name, newval)));
! return false;
! }
if (changeVal || makeDefault)
{
--- 4204,4211 ----
struct config_real *conf = (struct config_real *) record;
double newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 4016,4086 ****
struct config_string *conf = (struct config_string *) record;
char *newval;
! if (value)
! {
! newval = guc_strdup(elevel, value);
! if (newval == NULL)
! return false;
! /*
! * The only sort of "parsing" check we need to do is
! * apply truncation if GUC_IS_NAME.
! */
! if (conf->gen.flags & GUC_IS_NAME)
! truncate_identifier(newval, strlen(newval), true);
! }
! else if (conf->reset_val)
! {
! /*
! * We could possibly avoid strdup here, but easier to make
! * this case work the same as the normal assignment case.
! */
! newval = guc_strdup(elevel, conf->reset_val);
! if (newval == NULL)
! return false;
! source = conf->gen.reset_source;
! }
! else
! {
! /* Nothing to reset to, as yet; so do nothing */
! break;
! }
!
! if (conf->assign_hook)
! {
! const char *hookresult;
!
! /*
! * If the hook ereports, we have to make sure we free
! * newval, else it will be a permanent memory leak.
! */
! hookresult = call_string_assign_hook(conf->assign_hook,
! newval,
! changeVal,
! source);
! if (hookresult == NULL)
! {
! free(newval);
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": \"%s\"",
! name, value ? value : "")));
! return false;
! }
! else if (hookresult != newval)
! {
! free(newval);
!
! /*
! * Having to cast away const here is annoying, but the
! * alternative is to declare assign_hooks as returning
! * char*, which would mean they'd have to cast away
! * const, or as both taking and returning char*, which
! * doesn't seem attractive either --- we don't want
! * them to scribble on the passed str.
! */
! newval = (char *) hookresult;
! }
! }
if (changeVal || makeDefault)
{
--- 4256,4263 ----
struct config_string *conf = (struct config_string *) record;
char *newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
***************
*** 4128,4134 ****
}
}
else
! free(newval);
break;
}
}
--- 4305,4312 ----
}
}
else
! if( newval != NULL )
! free(newval);
break;
}
}
***************
*** 5130,5135 ****
--- 5308,5318 ----
static bool
is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ if( !newvalue )
+ {
+ return false;
+ }
+
switch (record->vartype)
{
case PGC_BOOL:
Index: src/include/utils/guc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
retrieving revision 1.70
diff -c -r1.70 guc.h
*** src/include/utils/guc.h 25 Jul 2006 03:51:22 -0000 1.70
--- src/include/utils/guc.h 27 Jul 2006 14:07:48 -0000
***************
*** 16,22 ****
#include "tcop/dest.h"
#include "utils/array.h"
-
/*
* Certain options can only be set at certain times. The rules are
* like this:
--- 16,21 ----
***************
*** 193,198 ****
--- 192,200 ----
extern bool set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
bool isLocal, bool changeVal);
+ extern bool verify_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool *isNewEqual, bool *isContextOK);
extern char *GetConfigOptionByName(const char *name, const char **varname);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.23
diff -c -r1.23 guc_tables.h
*** src/include/utils/guc_tables.h 13 Jul 2006 16:49:20 -0000 1.23
--- src/include/utils/guc_tables.h 27 Jul 2006 14:07:48 -0000
***************
*** 134,140 ****
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
!
/* GUC records for specific variable types */
--- 134,141 ----
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
! #define GUC_IN_CONFFILE 0x0008 /* value shows up in the configuration
! file (is not commented) */
/* GUC records for specific variable types */
***************
*** 144,154 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool reset_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
};
struct config_int
--- 145,156 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool boot_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
+ bool reset_val;
};
struct config_int
***************
*** 157,169 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int reset_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
};
struct config_real
--- 159,172 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int boot_val;
int min;
int max;
GucIntAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
+ int reset_val;
};
struct config_real
***************
*** 172,184 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double reset_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
};
struct config_string
--- 175,189 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double boot_val;
double min;
double max;
GucRealAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
+ double reset_val;
+
};
struct config_string
Zdenek Kotala wrote: > I performed some cleanup in my code as well. I reduced some > conditions, which cannot occur and fixed context validation in the > set_config_options function. I hope that It is final version of our > patch. The way I see it, combining a feature change with a code refactoring and random white space changes is a pretty optimal way to get your patch rejected. Please submit patches for these items separately. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote:
>
> The way I see it, combining a feature change with a code refactoring and
> random white space changes is a pretty optimal way to get your patch
> rejected. Please submit patches for these items separately.
>
OK. I split patch to two parts. Part one is refactoring of
set_config_options function. Part two implements feature "Allow
commenting of variables in postgresql.conf to restore them to defaults".
Zdenek
diff -r -c pgsql/src/backend/utils/misc/guc-file.l pgsql_1/src/backend/utils/misc/guc-file.l
*** pgsql/src/backend/utils/misc/guc-file.l Thu Jul 27 10:30:41 2006
--- pgsql_1/src/backend/utils/misc/guc-file.l Wed Aug 2 13:35:27 2006
***************
*** 50,56 ****
static bool ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, 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);
--- 50,57 ----
static bool ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, int elevel,
struct name_value_pair **head_p,
! struct name_value_pair **tail_p,
! int *varcount);
static void free_name_value_list(struct name_value_pair * list);
static char *GUC_scanstr(const char *s);
***************
*** 114,121 ****
void
ProcessConfigFile(GucContext context)
{
! int elevel;
struct name_value_pair *item, *head, *tail;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
--- 115,124 ----
void
ProcessConfigFile(GucContext context)
{
! int elevel, i;
struct name_value_pair *item, *head, *tail;
+ bool *apply_list = NULL;
+ int varcount = 0;
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
***************
*** 134,158 ****
if (!ParseConfigFile(ConfigFileName, NULL,
0, context, elevel,
! &head, &tail))
goto cleanup_list;
/* Check if all options are valid */
! for (item = head; item; item = item->next)
{
! if (!set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, false))
goto cleanup_list;
}
/* If we got here all the options checked out okay, so apply them. */
! for (item = head; item; item = item->next)
! {
! set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, true);
! }
! cleanup_list:
free_name_value_list(head);
}
--- 137,192 ----
if (!ParseConfigFile(ConfigFileName, NULL,
0, context, elevel,
! &head, &tail, &varcount))
goto cleanup_list;
+ /* Can we allocate memory here, what about leaving here prematurely? */
+ apply_list = (bool *) palloc(sizeof(bool) * varcount);
+
/* Check if all options are valid */
! for (item = head, i = 0; item; item = item->next, i++)
{
! bool isEqual, isContextOk;
!
! if (!verify_config_option(item->name, item->value, context,
! PGC_S_FILE, &isEqual, &isContextOk))
! {
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("configuration file is invalid")));
goto cleanup_list;
+ }
+
+ if( isContextOk == false )
+ {
+ apply_list[i] = false;
+ if( context == PGC_SIGHUP )
+ {
+ if ( isEqual == false )
+ ereport(elevel,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
+ item->name)));
+ }
+ else
+ /* if it is boot phase, context must be valid for all
+ * configuration item. */
+ goto cleanup_list;
+ }
+ else
+ apply_list[i] = true;
}
/* If we got here all the options checked out okay, so apply them. */
! for (item = head, i = 0; item; item = item->next, i++)
! if (apply_list[i])
! set_config_option(item->name, item->value, context,
! PGC_S_FILE, false, true);
!
! cleanup_list:
! if (apply_list)
! pfree(apply_list);
free_name_value_list(head);
}
***************
*** 189,201 ****
ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, 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
--- 223,236 ----
ParseConfigFile(const char *config_file, const char *calling_file,
int depth, GucContext context, int elevel,
struct name_value_pair **head_p,
! struct name_value_pair **tail_p,
! int *varcount)
{
! 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
***************
*** 289,295 ****
if (!ParseConfigFile(opt_value, config_file,
depth + 1, context, elevel,
! head_p, tail_p))
{
pfree(opt_name);
pfree(opt_value);
--- 324,330 ----
if (!ParseConfigFile(opt_value, config_file,
depth + 1, context, elevel,
! head_p, tail_p, varcount))
{
pfree(opt_name);
pfree(opt_value);
***************
*** 333,338 ****
--- 368,374 ----
else
(*tail_p)->next = item;
*tail_p = item;
+ (*varcount)++;
}
/* break out of loop if read EOF, else loop for next line */
diff -r -c pgsql/src/backend/utils/misc/guc.c pgsql_1/src/backend/utils/misc/guc.c
*** pgsql/src/backend/utils/misc/guc.c Sat Jul 29 05:02:56 2006
--- pgsql_1/src/backend/utils/misc/guc.c Wed Aug 2 14:32:14 2006
***************
*** 3690,3785 ****
return result;
}
-
/*
! * Sets option `name' to given value. The value should be a string
! * which is going to be parsed and converted to the appropriate data
! * type. The context and source parameters indicate in which context this
! * function is being called so it can apply the access restrictions
! * properly.
! *
! * If value is NULL, set the option to its default value. If the
! * parameter changeVal is false then don't really set the option but do all
! * the checks to see if it would work.
! *
! * If there is an error (non-existing option, invalid value) then an
! * ereport(ERROR) is thrown *unless* this is called in a context where we
! * don't want to ereport (currently, startup or SIGHUP config file reread).
! * In that case we write a suitable error message via ereport(DEBUG) and
! * return false. This is working around the deficiencies in the ereport
! * mechanism, so don't blame me. In all other cases, the function
! * returns true, including cases where the input is valid but we chose
! * not to apply it because of context or source-priority considerations.
! *
! * See also SetConfigOption for an external interface.
*/
! bool
! set_config_option(const char *name, const char *value,
! GucContext context, GucSource source,
! bool isLocal, bool changeVal)
{
- struct config_generic *record;
- int elevel;
- bool makeDefault;
! if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
{
! /*
! * To avoid cluttering the log, only the postmaster bleats loudly
! * about problems with the config file.
! */
! elevel = IsUnderPostmaster ? DEBUG2 : LOG;
! }
! else if (source == PGC_S_DATABASE || source == PGC_S_USER)
! elevel = INFO;
! else
! elevel = ERROR;
! record = find_option(name, elevel);
! if (record == NULL)
! {
! ereport(elevel,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("unrecognized configuration parameter \"%s\"", name)));
! return false;
}
! /*
! * Check if the option can be set at this time. See guc.h for the precise
! * rules. Note that we don't want to throw errors if we're in the SIGHUP
! * context. In that case we just ignore the attempt and return true.
! */
switch (record->context)
{
case PGC_INTERNAL:
- if (context == PGC_SIGHUP)
- return true;
if (context != PGC_INTERNAL)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed",
! name)));
return false;
}
break;
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
! {
! if (changeVal && !is_newvalue_equal(record, value))
! ereport(elevel,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("parameter \"%s\" cannot be changed after server start; configuration file change
ignored",
! name)));
- return true;
- }
if (context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start",
! name)));
return false;
}
break;
--- 3690,3947 ----
return result;
}
/*
! * Try to parse value. Determine what is type and call related
! * parsing function or if newval is equal to NULL, reset value
! * to default or bootval. If the value parsed okay return true,
! * else false.
*/
! static bool
! parse_value(int elevel, const struct config_generic *record,
! const char *value, GucSource *source, bool changeVal,
! union config_var_value *retval)
{
! Assert( !(changeVal && retval==NULL) );
! /*
! * Evaluate value and set variable.
! */
! switch (record->vartype)
{
! case PGC_BOOL:
! {
! struct config_bool *conf = (struct config_bool *) record;
! bool newval;
! if (value)
! {
! if (!parse_bool(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a Boolean value",
! record->name)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! record->name, (int) newval)));
! return false;
! }
! if( retval != NULL )
! retval->boolval = newval;
! break;
! }
!
! case PGC_INT:
! {
! struct config_int *conf = (struct config_int *) record;
! int newval;
!
! if (value)
! {
! if (!parse_int(value, &newval, conf->gen.flags))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires an integer value",
! record->name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
! newval, record->name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %d",
! record->name, newval)));
! return false;
! }
! if( retval != NULL )
! retval->intval = newval;
! break;
! }
!
! case PGC_REAL:
! {
! struct config_real *conf = (struct config_real *) record;
! double newval;
!
! if (value)
! {
! if (!parse_real(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a numeric value",
! record->name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
! newval, record->name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
!
! if (conf->assign_hook)
! if (!(*conf->assign_hook) (newval, changeVal, *source))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": %g",
! record->name, newval)));
! return false;
! }
! if( retval != NULL )
! retval->realval = newval;
! break;
! }
!
! case PGC_STRING:
! {
! struct config_string *conf = (struct config_string *) record;
! char *newval;
!
! if (value)
! {
! newval = guc_strdup(elevel, value);
! if (newval == NULL)
! return false;
! /*
! * The only sort of "parsing" check we need to do is
! * apply truncation if GUC_IS_NAME.
! */
! if (conf->gen.flags & GUC_IS_NAME)
! truncate_identifier(newval, strlen(newval), true);
! }
! else if (conf->reset_val)
! {
! /*
! * We could possibly avoid strdup here, but easier to make
! * this case work the same as the normal assignment case.
! */
! newval = guc_strdup(elevel, conf->reset_val);
! if (newval == NULL)
! return false;
! *source = conf->gen.reset_source;
! }
! else
! {
! /* Nothing to reset to, as yet; so do nothing */
! break;
! }
!
! if (conf->assign_hook)
! {
! const char *hookresult;
!
! /*
! * If the hook ereports, we have to make sure we free
! * newval, else it will be a permanent memory leak.
! */
! hookresult = call_string_assign_hook(conf->assign_hook,
! newval,
! changeVal,
! *source);
! if (hookresult == NULL)
! {
! free(newval);
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("invalid value for parameter \"%s\": \"%s\"",
! record->name, value ? value : "")));
! return false;
! }
! else if (hookresult != newval)
! {
! free(newval);
!
! /*
! * Having to cast away const here is annoying, but the
! * alternative is to declare assign_hooks as returning
! * char*, which would mean they'd have to cast away
! * const, or as both taking and returning char*, which
! * doesn't seem attractive either --- we don't want
! * them to scribble on the passed str.
! */
! newval = (char *) hookresult;
! }
! }
!
! if ( !changeVal )
! free(newval);
! if( retval != NULL )
! retval->stringval= newval;
! break;
! }
}
+ return true;
+ }
! /*
! * Check if the option can be set at this time. See guc.h for the precise
! * rules.
! */
! static bool
! checkContext(int elevel, struct config_generic *record, GucContext context)
! {
switch (record->context)
{
case PGC_INTERNAL:
if (context != PGC_INTERNAL)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed",
! record->name)));
return false;
}
break;
case PGC_POSTMASTER:
if (context == PGC_SIGHUP)
! return false;
if (context != PGC_POSTMASTER)
{
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed after server start",
! record->name)));
return false;
}
break;
***************
*** 3789,3795 ****
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed now",
! name)));
return false;
}
--- 3951,3957 ----
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed now",
! record->name)));
return false;
}
***************
*** 3812,3818 ****
* backend start.
*/
if (IsUnderPostmaster)
! return true;
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
--- 3974,3982 ----
* backend start.
*/
if (IsUnderPostmaster)
! {
! return false;
! }
}
else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
{
***************
*** 3819,3825 ****
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be set after connection start",
! name)));
return false;
}
break;
--- 3983,3989 ----
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be set after connection start",
! record->name)));
return false;
}
break;
***************
*** 3829,3835 ****
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set parameter \"%s\"",
! name)));
return false;
}
break;
--- 3993,3999 ----
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to set parameter \"%s\"",
! record->name)));
return false;
}
break;
***************
*** 3837,3843 ****
--- 4001,4116 ----
/* always okay */
break;
}
+ return true;
+ }
+ /*
+ * Get error level for different sources and context.
+ */
+ static int
+ get_elevel(GucContext context, GucSource source)
+ {
+ int elevel;
+ if (context == PGC_SIGHUP || source == PGC_S_DEFAULT)
+ {
+ /*
+ * To avoid cluttering the log, only the postmaster bleats loudly
+ * about problems with the config file.
+ */
+ elevel = IsUnderPostmaster ? DEBUG2 : LOG;
+ }
+ else if (source == PGC_S_DATABASE || source == PGC_S_USER)
+ elevel = INFO;
+ else
+ elevel = ERROR;
+
+ return elevel;
+ }
+
+ /*
+ * Verify if option exists and value is valid.
+ * It is primary used for validation of items in configuration file.
+ */
+ bool
+ verify_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool *isNewEqual, bool *isContextOK)
+ {
+ union config_var_value newval;
+ int elevel;
+ struct config_generic *record;
+
+ elevel = get_elevel(context, source);
+
+ record = find_option(name, elevel);
+ if (record == NULL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ return false;
+ }
+
+ if( parse_value(elevel, record, value, &source, false, &newval) )
+ {
+ if( isNewEqual != NULL)
+ *isNewEqual = is_newvalue_equal(record, value);
+ if( isContextOK != NULL)
+ *isContextOK = checkContext(elevel, record, context);
+ }
+ else
+ return false;
+
+ return true;
+ }
+
+
+ /*
+ * Sets option `name' to given value. The value should be a string
+ * which is going to be parsed and converted to the appropriate data
+ * type. The context and source parameters indicate in which context this
+ * function is being called so it can apply the access restrictions
+ * properly.
+ *
+ * If value is NULL, set the option to its default value. If the
+ * parameter changeVal is false then don't really set the option but do all
+ * the checks to see if it would work.
+ *
+ * If there is an error (non-existing option, invalid value) then an
+ * ereport(ERROR) is thrown *unless* this is called in a context where we
+ * don't want to ereport (currently, startup or SIGHUP config file reread).
+ * In that case we write a suitable error message via ereport(DEBUG) and
+ * return false. This is working around the deficiencies in the ereport
+ * mechanism, so don't blame me. In all other cases, the function
+ * returns true, including cases where the input is valid but we chose
+ * not to apply it because of context or source-priority considerations.
+ *
+ * See also SetConfigOption for an external interface.
+ */
+ bool
+ set_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool isLocal, bool changeVal)
+ {
+ struct config_generic *record;
+ int elevel;
+ bool makeDefault;
+
+ elevel = get_elevel(context, source);
+
+ record = find_option(name, elevel);
+ if (record == NULL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ return false;
+ }
+
+ /* Check if change is allowed in the running context. */
+ if( !checkContext(elevel, record, context) )
+ return false;
+
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
***************
*** 3871,3904 ****
{
struct config_bool *conf = (struct config_bool *) record;
bool newval;
- if (value)
- {
- if (!parse_bool(value, &newval))
- {
- ereport(elevel,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value",
- name)));
- return false;
- }
- }
- else
- {
- newval = conf->reset_val;
- source = conf->gen.reset_source;
- }
-
- if (conf->assign_hook)
- if (!(*conf->assign_hook) (newval, changeVal, source))
- {
- ereport(elevel,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for parameter \"%s\": %d",
- name, (int) newval)));
- return false;
- }
-
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
--- 4144,4153 ----
{
struct config_bool *conf = (struct config_bool *) record;
bool newval;
+
+ if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
+ return false;
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
***************
*** 3948,3988 ****
struct config_int *conf = (struct config_int *) record;
int newval;
! if (value)
! {
! if (!parse_int(value, &newval, conf->gen.flags))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires an integer value",
! name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)",
! newval, name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
- if (conf->assign_hook)
- if (!(*conf->assign_hook) (newval, changeVal, source))
- {
- ereport(elevel,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for parameter \"%s\": %d",
- name, newval)));
- return false;
- }
-
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
--- 4197,4205 ----
struct config_int *conf = (struct config_int *) record;
int newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
***************
*** 4032,4072 ****
struct config_real *conf = (struct config_real *) record;
double newval;
! if (value)
! {
! if (!parse_real(value, &newval))
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" requires a numeric value",
! name)));
! return false;
! }
! if (newval < conf->min || newval > conf->max)
! {
! ereport(elevel,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)",
! newval, name, conf->min, conf->max)));
! return false;
! }
! }
! else
! {
! newval = conf->reset_val;
! source = conf->gen.reset_source;
! }
- if (conf->assign_hook)
- if (!(*conf->assign_hook) (newval, changeVal, source))
- {
- ereport(elevel,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for parameter \"%s\": %g",
- name, newval)));
- return false;
- }
-
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
--- 4249,4257 ----
struct config_real *conf = (struct config_real *) record;
double newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
***************
*** 4116,4187 ****
struct config_string *conf = (struct config_string *) record;
char *newval;
! if (value)
! {
! newval = guc_strdup(elevel, value);
! if (newval == NULL)
! return false;
! /*
! * The only sort of "parsing" check we need to do is
! * apply truncation if GUC_IS_NAME.
! */
! if (conf->gen.flags & GUC_IS_NAME)
! truncate_identifier(newval, strlen(newval), true);
! }
! else if (conf->reset_val)
! {
! /*
! * We could possibly avoid strdup here, but easier to make
! * this case work the same as the normal assignment case.
! */
! newval = guc_strdup(elevel, conf->reset_val);
! if (newval == NULL)
! return false;
! source = conf->gen.reset_source;
! }
! else
! {
! /* Nothing to reset to, as yet; so do nothing */
! break;
! }
- if (conf->assign_hook)
- {
- const char *hookresult;
-
- /*
- * If the hook ereports, we have to make sure we free
- * newval, else it will be a permanent memory leak.
- */
- hookresult = call_string_assign_hook(conf->assign_hook,
- newval,
- changeVal,
- source);
- if (hookresult == NULL)
- {
- free(newval);
- ereport(elevel,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for parameter \"%s\": \"%s\"",
- name, value ? value : "")));
- return false;
- }
- else if (hookresult != newval)
- {
- free(newval);
-
- /*
- * Having to cast away const here is annoying, but the
- * alternative is to declare assign_hooks as returning
- * char*, which would mean they'd have to cast away
- * const, or as both taking and returning char*, which
- * doesn't seem attractive either --- we don't want
- * them to scribble on the passed str.
- */
- newval = (char *) hookresult;
- }
- }
-
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
--- 4301,4309 ----
struct config_string *conf = (struct config_string *) record;
char *newval;
! if( !parse_value(elevel, record, value, &source, changeVal, (union config_var_value*) &newval) )
! return false;
if (changeVal || makeDefault)
{
/* Save old value to support transaction abort */
***************
*** 4228,4234 ****
}
}
else
! free(newval);
break;
}
}
--- 4350,4357 ----
}
}
else
! if( newval != NULL )
! free(newval);
break;
}
}
***************
*** 5314,5319 ****
--- 5437,5445 ----
static bool
is_newvalue_equal(struct config_generic *record, const char *newvalue)
{
+ if( !newvalue )
+ return false;
+
switch (record->vartype)
{
case PGC_BOOL:
diff -r -c pgsql/src/include/utils/guc.h pgsql_1/src/include/utils/guc.h
*** pgsql/src/include/utils/guc.h Sat Jul 29 05:02:56 2006
--- pgsql_1/src/include/utils/guc.h Wed Aug 2 13:49:22 2006
***************
*** 193,198 ****
--- 193,201 ----
extern bool set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
bool isLocal, bool changeVal);
+ extern bool verify_config_option(const char *name, const char *value,
+ GucContext context, GucSource source,
+ bool *isNewEqual, bool *isContextOK);
extern char *GetConfigOptionByName(const char *name, const char **varname);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
diff -r -c pgsql_1/src/backend/utils/misc/guc-file.l pgsql_2/src/backend/utils/misc/guc-file.l
*** pgsql_1/src/backend/utils/misc/guc-file.l Wed Aug 2 13:35:27 2006
--- pgsql_2/src/backend/utils/misc/guc-file.l Wed Aug 2 15:24:54 2006
***************
*** 117,122 ****
--- 117,123 ----
{
int elevel, i;
struct name_value_pair *item, *head, *tail;
+ char *env;
bool *apply_list = NULL;
int varcount = 0;
***************
*** 183,189 ****
--- 184,242 ----
set_config_option(item->name, item->value, context,
PGC_S_FILE, false, true);
+ if( context == PGC_SIGHUP)
+ {
+ /*
+ * Revert all "untouched" options with reset source PGC_S_FILE to
+ * default/boot value.
+ */
+ for (i = 0; i < num_guc_variables; i++)
+ {
+ struct config_generic *gconf = guc_variables[i];
+ if ( gconf->reset_source == PGC_S_FILE &&
+ !(gconf->status & GUC_IN_CONFFILE) )
+ {
+ if ( gconf->context == PGC_BACKEND && IsUnderPostmaster)
+ ; /* Be silent. Does any body want message from each session? */
+ else if (gconf->context == PGC_POSTMASTER)
+ ereport(elevel,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed (commented) after server start; configuration file
changeignored",
+ gconf->name)));
+ else if(set_config_option(gconf->name,
+ NULL, context,
+ PGC_S_FILE,
+ false, true))
+ {
+ GucStack *stack;
+ /* set correctly source */
+ gconf->reset_source = PGC_S_DEFAULT;
+ for (stack = gconf->stack; stack; stack = stack->prev)
+ if (stack->source == PGC_S_FILE)
+ stack->source = PGC_S_DEFAULT;
+ ereport(elevel,
+ (errcode(ERRCODE_SUCCESSFUL_COMPLETION),
+ errmsg("configuration option %s falls back to default value", gconf->name)));
+ }
+ }
+ gconf->status &= ~GUC_IN_CONFFILE;
+ }
+
+ /* Revert to environment variable. PGPORT is ignored, because it cannot be
+ * set in running state.
+ */
+ env = getenv("PGDATESTYLE");
+ if (env != NULL)
+ set_config_option("datestyle", env, context,
+ PGC_S_ENV_VAR, false, true);
+
+ env = getenv("PGCLIENTENCODING");
+ if (env != NULL)
+ set_config_option("client_encoding", env, context,
+ PGC_S_ENV_VAR, false, true);
+ }
+
cleanup_list:
if (apply_list)
pfree(apply_list);
diff -r -c pgsql_1/src/backend/utils/misc/guc.c pgsql_2/src/backend/utils/misc/guc.c
*** pgsql_1/src/backend/utils/misc/guc.c Wed Aug 2 14:32:14 2006
--- pgsql_2/src/backend/utils/misc/guc.c Wed Aug 2 15:25:00 2006
***************
*** 2694,2704 ****
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_INT:
--- 2694,2704 ----
struct config_bool *conf = (struct config_bool *) gconf;
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, (int) conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_INT:
***************
*** 2705,2718 ****
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_REAL:
--- 2705,2718 ----
{
struct config_int *conf = (struct config_int *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %d",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_REAL:
***************
*** 2719,2732 ****
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->reset_val >= conf->min);
! Assert(conf->reset_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->reset_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->reset_val);
! *conf->variable = conf->reset_val;
break;
}
case PGC_STRING:
--- 2719,2732 ----
{
struct config_real *conf = (struct config_real *) gconf;
! Assert(conf->boot_val >= conf->min);
! Assert(conf->boot_val <= conf->max);
if (conf->assign_hook)
! if (!(*conf->assign_hook) (conf->boot_val, true,
PGC_S_DEFAULT))
elog(FATAL, "failed to initialize %s to %g",
! conf->gen.name, conf->boot_val);
! *conf->variable = conf->reset_val = conf->boot_val;
break;
}
case PGC_STRING:
***************
*** 3179,3185 ****
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status;
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
--- 3179,3185 ----
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *gconf = guc_variables[i];
! int my_status = gconf->status & (~GUC_IN_CONFFILE);
GucStack *stack = gconf->stack;
bool useTentative;
bool changed;
***************
*** 3726,3733 ****
}
else
{
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3726,3744 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3770,3777 ****
}
else
{
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3781,3799 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3814,3821 ****
}
else
{
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
}
if (conf->assign_hook)
--- 3836,3854 ----
}
else
{
! /* Revert value to default if source is configuration file. It is used when
! * configuration parameter is removed/commented out in the config file. Else
! * RESET or SET TO DEFAULT command is called and reset_val is used.
! */
! if( *source == PGC_S_FILE )
! {
! newval = conf->boot_val;
! }
! else
! {
! newval = conf->reset_val;
! *source = conf->gen.reset_source;
! }
}
if (conf->assign_hook)
***************
*** 3849,3854 ****
--- 3882,3901 ----
if (conf->gen.flags & GUC_IS_NAME)
truncate_identifier(newval, strlen(newval), true);
}
+ else if (*source == PGC_S_FILE)
+ {
+ /* Revert value to default when item is removed from config file. */
+ if ( conf->boot_val != NULL )
+ {
+ newval = guc_strdup(elevel, conf->boot_val);
+ if (newval == NULL)
+ return false;
+ }
+ else
+ {
+ return false;
+ }
+ }
else if (conf->reset_val)
{
/*
***************
*** 4053,4058 ****
--- 4100,4110 ----
if( parse_value(elevel, record, value, &source, false, &newval) )
{
+ /* Mark record like presented in the config file. Be carefull if
+ * you use this function for another purpose than config file
+ * verification. It causes confusion configfile parser. */
+ record->status |= GUC_IN_CONFFILE;
+
if( isNewEqual != NULL)
*isNewEqual = is_newvalue_equal(record, value);
if( isContextOK != NULL)
***************
*** 4115,4121 ****
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
/*
* Ignore attempted set if overridden by previously processed setting.
--- 4167,4173 ----
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)
*/
! makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
/*
* Ignore attempted set if overridden by previously processed setting.
diff -r -c pgsql_1/src/include/utils/guc_tables.h pgsql_2/src/include/utils/guc_tables.h
*** pgsql_1/src/include/utils/guc_tables.h Thu Jul 27 10:30:41 2006
--- pgsql_2/src/include/utils/guc_tables.h Wed Aug 2 15:31:09 2006
***************
*** 143,150 ****
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
-
/* GUC records for specific variable types */
struct config_bool
--- 143,151 ----
#define GUC_HAVE_TENTATIVE 0x0001 /* tentative value is defined */
#define GUC_HAVE_LOCAL 0x0002 /* a SET LOCAL has been executed */
#define GUC_HAVE_STACK 0x0004 /* we have stacked prior value(s) */
+ #define GUC_IN_CONFFILE 0x0008 /* value shows up in the configuration
+ file (is not commented) */
/* GUC records for specific variable types */
struct config_bool
***************
*** 153,163 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool reset_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
};
struct config_int
--- 154,165 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
bool *variable;
! bool boot_val;
GucBoolAssignHook assign_hook;
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
bool tentative_val;
+ bool reset_val;
};
struct config_int
***************
*** 166,172 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int reset_val;
int min;
int max;
GucIntAssignHook assign_hook;
--- 168,174 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
int *variable;
! int boot_val;
int min;
int max;
GucIntAssignHook assign_hook;
***************
*** 173,178 ****
--- 175,181 ----
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
int tentative_val;
+ int reset_val;
};
struct config_real
***************
*** 181,187 ****
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double reset_val;
double min;
double max;
GucRealAssignHook assign_hook;
--- 184,190 ----
/* these fields must be set correctly in initial value: */
/* (all but reset_val are constants) */
double *variable;
! double boot_val;
double min;
double max;
GucRealAssignHook assign_hook;
***************
*** 188,193 ****
--- 191,197 ----
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
double tentative_val;
+ double reset_val;
};
struct config_string
Zdenek Kotala wrote:
> OK. I split patch to two parts. Part one is refactoring of
> set_config_options function. Part two implements feature "Allow
> commenting of variables in postgresql.conf to restore them to
> defaults".
I'm having trouble wrapping my head around a code "refactoring" which
actually makes the code significantly *longer*. The only interface
change I could detect is the introduction of a function
verify_config_option(), which should just be a small variation on
set_config_option() as it currently exists.
I'm also about a relive a personal trauma if I see error messages like
this:
errmsg("configuration file is invalid")
I just had to deal with an unnamed product where this was all you got!
Please, explain again what this refactoring is supposed to achieve.
The second part of your patch actually looks pretty reasonable and does
not appear to require the refactoring.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
Peter Eisentraut napsal(a):
> Zdenek Kotala wrote:
>> OK. I split patch to two parts. Part one is refactoring of
>> set_config_options function. Part two implements feature "Allow
>> commenting of variables in postgresql.conf to restore them to
>> defaults".
>
> I'm having trouble wrapping my head around a code "refactoring" which
> actually makes the code significantly *longer*. The only interface
> change I could detect is the introduction of a function
> verify_config_option(), which should just be a small variation on
> set_config_option() as it currently exists.
The main reason for refactoring was that set_config_option() was too
overloaded function and its behavior did not consistent. Old version of
set_config_function hides some messages. For example if you type:
tcp_port = 5432.1
then old implementation ignore this error without any message to log
file in the signal context (configuration reload). Main problem was that
semantic analysis of postgresql.conf is not perform in the
ProcessConfigFile function, but in the set_config_options *after*
context check. This skipped check for variables with PG_POSTMASTER
context. There was request from Joachim Wieland to add more messages
about ignored changes in the config file as well.
> I'm also about a relive a personal trauma if I see error messages like
> this:
>
> errmsg("configuration file is invalid")
>
> I just had to deal with an unnamed product where this was all you got!
Do you have any idea for better message? This message is last one during
parsing process. What is wrong is logged before this message.
Zdenek
Patch applied. Thanks. --------------------------------------------------------------------------- Zdenek Kotala wrote: > Peter Eisentraut wrote: > > > > The way I see it, combining a feature change with a code refactoring and > > random white space changes is a pretty optimal way to get your patch > > rejected. Please submit patches for these items separately. > > > > OK. I split patch to two parts. Part one is refactoring of > set_config_options function. Part two implements feature "Allow > commenting of variables in postgresql.conf to restore them to defaults". > > Zdenek -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Peter,
What is the status of this patch now? I read that two bugs has been
fixed in this patch and now it is waiting for new review. Is there
something what I can/must do?
Zdenek
Peter Eisentraut wrote:
> Zdenek Kotala wrote:
>> OK. I split patch to two parts. Part one is refactoring of
>> set_config_options function. Part two implements feature "Allow
>> commenting of variables in postgresql.conf to restore them to
>> defaults".
>
> I'm having trouble wrapping my head around a code "refactoring" which
> actually makes the code significantly *longer*. The only interface
> change I could detect is the introduction of a function
> verify_config_option(), which should just be a small variation on
> set_config_option() as it currently exists.
>
> I'm also about a relive a personal trauma if I see error messages like
> this:
>
> errmsg("configuration file is invalid")
>
> I just had to deal with an unnamed product where this was all you got!
>
> Please, explain again what this refactoring is supposed to achieve.
>
> The second part of your patch actually looks pretty reasonable and does
> not appear to require the refactoring.
>
Zdenek Kotala wrote:
> Peter,
>
> What is the status of this patch now? I read that two bugs has been
> fixed in this patch and now it is waiting for new review. Is there
> something what I can/must do?
The patch was applied, fixed, and fixed again, then reverted. It will
sit until just before beta, where it will be applied or a vote will be
taken on whether to apply it. There is nothing more you have to do on
it. I think the fixes just scared some folks so there is hope more
review might happen before beta.
---------------------------------------------------------------------------
> Zdenek
>
> Peter Eisentraut wrote:
> > Zdenek Kotala wrote:
> >> OK. I split patch to two parts. Part one is refactoring of
> >> set_config_options function. Part two implements feature "Allow
> >> commenting of variables in postgresql.conf to restore them to
> >> defaults".
> >
> > I'm having trouble wrapping my head around a code "refactoring" which
> > actually makes the code significantly *longer*. The only interface
> > change I could detect is the introduction of a function
> > verify_config_option(), which should just be a small variation on
> > set_config_option() as it currently exists.
> >
> > I'm also about a relive a personal trauma if I see error messages like
> > this:
> >
> > errmsg("configuration file is invalid")
> >
> > I just had to deal with an unnamed product where this was all you got!
> >
> > Please, explain again what this refactoring is supposed to achieve.
> >
> > The second part of your patch actually looks pretty reasonable and does
> > not appear to require the refactoring.
> >
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Am Mittwoch, 23. August 2006 14:44 schrieb Zdenek Kotala: > What is the status of this patch now? I read that two bugs has been > fixed in this patch and now it is waiting for new review. Is there > something what I can/must do? As I said previously, if you are "refactoring" code to make it longer, then I don't believe in the merit of the patch. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote:
> Am Mittwoch, 23. August 2006 14:44 schrieb Zdenek Kotala:
> > What is the status of this patch now? I read that two bugs has been
> > fixed in this patch and now it is waiting for new review. Is there
> > something what I can/must do?
>
> As I said previously, if you are "refactoring" code to make it longer, then I
> don't believe in the merit of the patch.
The refactoring was to fix problems in incorrect parsing of the config
file. I believe the example was:
http://archives.postgresql.org/pgsql-committers/2006-08/msg00245.php
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +