Re: Allow commenting of variables in postgresql.conf to - - Mailing list pgsql-patches

From Zdenek Kotala
Subject Re: Allow commenting of variables in postgresql.conf to -
Date
Msg-id 44D0B4C5.2010606@sun.com
Whole thread Raw
In response to Re: Allow commenting of variables in postgresql.conf to -  (Peter Eisentraut <peter_e@gmx.net>)
Responses Re: Allow commenting of variables in postgresql.conf to -
Re: Allow commenting of variables in postgresql.conf to -
List pgsql-patches
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

pgsql-patches by date:

Previous
From: Joe Conway
Date:
Subject: Re: Values list-of-targetlists patch for comments (was Re:
Next
From: Tom Lane
Date:
Subject: Re: Values list-of-targetlists patch for comments (was Re: