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

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