Thread: reset all update

reset all update

From
Marko Kreen
Date:
* GUCify command line arguments.
* ResetAllOptions() comment
* split set_config_option()
* use set_config_option_real() on string reset
  - it calls hooks and free()'s the previous val
* check if string val changed on reset

--
marko


Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.218
diff -u -c -r1.218 postmaster.c
*** src/backend/postmaster/postmaster.c    2001/06/11 04:12:29    1.218
--- src/backend/postmaster/postmaster.c    2001/06/11 09:17:01
***************
*** 422,435 ****
  #ifndef USE_ASSERT_CHECKING
                  postmaster_error("Assert checking is not compiled in.");
  #else
!                 assert_enabled = atoi(optarg);
  #endif
                  break;
              case 'a':
                  /* Can no longer set authentication method. */
                  break;
              case 'B':
!                 NBuffers = atoi(optarg);
                  break;
              case 'b':
                  /* Can no longer set the backend executable file to use. */
--- 422,435 ----
  #ifndef USE_ASSERT_CHECKING
                  postmaster_error("Assert checking is not compiled in.");
  #else
!                 SetConfigOption("debug_assertions", optarg, PGC_POSTMASTER, true);
  #endif
                  break;
              case 'a':
                  /* Can no longer set authentication method. */
                  break;
              case 'B':
!                 SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, true);
                  break;
              case 'b':
                  /* Can no longer set the backend executable file to use. */
***************
*** 443,465 ****
                   * Turn on debugging for the postmaster and the backend
                   * servers descended from it.
                   */
!                 DebugLvl = atoi(optarg);
                  break;
              case 'F':
!                 enableFsync = false;
                  break;
              case 'h':
!                 VirtualHost = optarg;
                  break;
              case 'i':
!                 NetServer = true;
                  break;
              case 'k':
!                 UnixSocketDir = optarg;
                  break;
  #ifdef USE_SSL
              case 'l':
!                 EnableSSL = true;
                  break;
  #endif
              case 'm':
--- 443,465 ----
                   * Turn on debugging for the postmaster and the backend
                   * servers descended from it.
                   */
!                 SetConfigOption("debug_level", optarg, PGC_POSTMASTER, true);
                  break;
              case 'F':
!                 SetConfigOption("enable_fsync", optarg, PGC_POSTMASTER, true);
                  break;
              case 'h':
!                 SetConfigOption("virtual_host", optarg, PGC_POSTMASTER, true);
                  break;
              case 'i':
!                 SetConfigOption("tcpip_socket", optarg, PGC_POSTMASTER, true);
                  break;
              case 'k':
!                 SetConfigOption("unix_socket_directory", optarg, PGC_POSTMASTER, true);
                  break;
  #ifdef USE_SSL
              case 'l':
!                 SetConfigOption("ssl", optarg, PGC_POSTMASTER, true);
                  break;
  #endif
              case 'm':
***************
*** 479,489 ****
                   * The max number of backends to start. Can't set to less
                   * than 1 or more than compiled-in limit.
                   */
!                 MaxBackends = atoi(optarg);
!                 if (MaxBackends < 1)
!                     MaxBackends = 1;
!                 if (MaxBackends > MAXBACKENDS)
!                     MaxBackends = MAXBACKENDS;
                  break;
              case 'n':
                  /* Don't reinit shared mem after abnormal exit */
--- 479,485 ----
                   * The max number of backends to start. Can't set to less
                   * than 1 or more than compiled-in limit.
                   */
!                 SetConfigOption("max_connections", optarg, PGC_POSTMASTER, true);
                  break;
              case 'n':
                  /* Don't reinit shared mem after abnormal exit */
***************
*** 500,506 ****
                  strcpy(original_extraoptions, optarg);
                  break;
              case 'p':
!                 PostPortNumber = atoi(optarg);
                  break;
              case 'S':

--- 496,502 ----
                  strcpy(original_extraoptions, optarg);
                  break;
              case 'p':
!                 SetConfigOption("port", optarg, PGC_POSTMASTER, true);
                  break;
              case 'S':

***************
*** 510,516 ****
                   * it's most badly needed on SysV-derived systems like
                   * SVR4 and HP-UX.
                   */
!                 SilentMode = true;
                  break;
              case 's':

--- 506,512 ----
                   * it's most badly needed on SysV-derived systems like
                   * SVR4 and HP-UX.
                   */
!                 SetConfigOption("silent_mode", optarg, PGC_POSTMASTER, true);
                  break;
              case 's':

Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.219
diff -u -c -r1.219 postgres.c
*** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
--- src/backend/tcop/postgres.c    2001/06/11 09:17:07
***************
*** 1108,1113 ****
--- 1108,1115 ----
      const char *DBName = NULL;
      bool        secure = true;
      int            errs = 0;
+     GucContext    ctx;
+     char        *tmp;

      int            firstchar;
      StringInfo    parser_input;
***************
*** 1117,1122 ****
--- 1119,1126 ----

      char       *potential_DataDir = NULL;

+     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
+
      /*
       * Catch standard options before doing much else.  This even works on
       * systems without getopt_long.
***************
*** 1188,1194 ****
          {
              case 'A':
  #ifdef USE_ASSERT_CHECKING
!                 assert_enabled = atoi(optarg);
  #else
                  fprintf(stderr, "Assert checking is not compiled in\n");
  #endif
--- 1192,1198 ----
          {
              case 'A':
  #ifdef USE_ASSERT_CHECKING
!                 SetConfigOption("debug_assertions", optarg, ctx, true);
  #else
                  fprintf(stderr, "Assert checking is not compiled in\n");
  #endif
***************
*** 1200,1206 ****
                   * specify the size of buffer pool
                   */
                  if (secure)
!                     NBuffers = atoi(optarg);
                  break;

              case 'C':
--- 1204,1210 ----
                   * specify the size of buffer pool
                   */
                  if (secure)
!                     SetConfigOption("shared_buffers", optarg, ctx, true);
                  break;

              case 'C':
***************
*** 1217,1233 ****
                  break;

              case 'd':            /* debug level */
!                 DebugLvl = atoi(optarg);
                  if (DebugLvl >= 1);
!                 Log_connections = true;
                  if (DebugLvl >= 2)
!                     Debug_print_query = true;
                  if (DebugLvl >= 3)
!                     Debug_print_parse = true;
                  if (DebugLvl >= 4)
!                     Debug_print_plan = true;
                  if (DebugLvl >= 5)
!                     Debug_print_rewritten = true;
                  break;

              case 'E':
--- 1221,1238 ----
                  break;

              case 'd':            /* debug level */
!                 tmp = "true";
!                 SetConfigOption("debug_level", optarg, ctx, true);
                  if (DebugLvl >= 1);
!                 SetConfigOption("log_connections", tmp, ctx, true);
                  if (DebugLvl >= 2)
!                     SetConfigOption("debug_print_query", tmp, ctx, true);
                  if (DebugLvl >= 3)
!                     SetConfigOption("debug_print_parse", tmp, ctx, true);
                  if (DebugLvl >= 4)
!                     SetConfigOption("debug_print_plan", tmp, ctx, true);
                  if (DebugLvl >= 5)
!                     SetConfigOption("debug_print_rewritten", tmp, ctx, true);
                  break;

              case 'E':
***************
*** 1252,1258 ****
                   * turn off fsync
                   */
                  if (secure)
!                     enableFsync = false;
                  break;

              case 'f':
--- 1257,1263 ----
                   * turn off fsync
                   */
                  if (secure)
!                     SetConfigOption("fsync", "true", ctx, true);
                  break;

              case 'f':
***************
*** 1260,1288 ****
                  /*
                   * f - forbid generation of certain plans
                   */
                  switch (optarg[0])
                  {
                      case 's':    /* seqscan */
!                         enable_seqscan = false;
                          break;
                      case 'i':    /* indexscan */
!                         enable_indexscan = false;
                          break;
                      case 't':    /* tidscan */
!                         enable_tidscan = false;
                          break;
                      case 'n':    /* nestloop */
!                         enable_nestloop = false;
                          break;
                      case 'm':    /* mergejoin */
!                         enable_mergejoin = false;
                          break;
                      case 'h':    /* hashjoin */
!                         enable_hashjoin = false;
                          break;
                      default:
                          errs++;
                  }
                  break;

              case 'i':
--- 1265,1296 ----
                  /*
                   * f - forbid generation of certain plans
                   */
+                 tmp = NULL;
                  switch (optarg[0])
                  {
                      case 's':    /* seqscan */
!                         tmp = "enable_seqscan";
                          break;
                      case 'i':    /* indexscan */
!                         tmp = "enable_indexscan";
                          break;
                      case 't':    /* tidscan */
!                         tmp = "enable_tidscan";
                          break;
                      case 'n':    /* nestloop */
!                         tmp = "enable_nestloop";
                          break;
                      case 'm':    /* mergejoin */
!                         tmp = "enable_mergejoin";
                          break;
                      case 'h':    /* hashjoin */
!                         tmp = "enable_hashjoin";
                          break;
                      default:
                          errs++;
                  }
+                 if (tmp)
+                     SetConfigOption(tmp, "false", ctx, true);
                  break;

              case 'i':
***************
*** 1352,1364 ****
                  /*
                   * S - amount of sort memory to use in 1k bytes
                   */
!                 {
!                     int            S;
!
!                     S = atoi(optarg);
!                     if (S >= 4 * BLCKSZ / 1024)
!                         SortMem = S;
!                 }
                  break;

              case 's':
--- 1360,1366 ----
                  /*
                   * S - amount of sort memory to use in 1k bytes
                   */
!                 SetConfigOption("sort_mem", optarg, ctx, true);
                  break;

              case 's':
***************
*** 1366,1372 ****
                  /*
                   * s - report usage statistics (timings) after each query
                   */
!                 Show_query_stats = 1;
                  break;

              case 't':
--- 1368,1374 ----
                  /*
                   * s - report usage statistics (timings) after each query
                   */
!                 SetConfigOption("show_query_stats", optarg, ctx, true);
                  break;

              case 't':
***************
*** 1380,1402 ****
                   *    caution: -s can not be used together with -t.
                   * ----------------
                   */
                  switch (optarg[0])
                  {
                      case 'p':
                          if (optarg[1] == 'a')
!                             Show_parser_stats = 1;
                          else if (optarg[1] == 'l')
!                             Show_planner_stats = 1;
                          else
                              errs++;
                          break;
                      case 'e':
!                         Show_executor_stats = 1;
                          break;
                      default:
                          errs++;
                          break;
                  }
                  break;

              case 'v':
--- 1382,1407 ----
                   *    caution: -s can not be used together with -t.
                   * ----------------
                   */
+                 tmp = NULL;
                  switch (optarg[0])
                  {
                      case 'p':
                          if (optarg[1] == 'a')
!                             tmp = "show_parser_stats";
                          else if (optarg[1] == 'l')
!                             tmp = "show_planner_stats";
                          else
                              errs++;
                          break;
                      case 'e':
!                         tmp = "show_parser_stats";
                          break;
                      default:
                          errs++;
                          break;
                  }
+                 if (tmp)
+                     SetConfigOption(tmp, "true", ctx, true);
                  break;

              case 'v':
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.37
diff -u -c -r1.37 guc.c
*** src/backend/utils/misc/guc.c    2001/06/07 04:50:57    1.37
--- src/backend/utils/misc/guc.c    2001/06/11 09:17:09
***************
*** 143,148 ****
--- 143,151 ----
  };


+ static bool set_config_option_real(enum config_type type, struct config_generic *record,
+                 const char *value, bool DoIt, bool makeDefault, int elevel);
+
  /*
   * TO ADD AN OPTION:
   *
***************
*** 264,270 ****
      DEF_PGPORT, 1, 65535},

      {"sort_mem", PGC_USERSET, &SortMem,
!     512, 1, INT_MAX},

      {"debug_level", PGC_USERSET, &DebugLvl,
      0, 0, 16},
--- 267,273 ----
      DEF_PGPORT, 1, 65535},

      {"sort_mem", PGC_USERSET, &SortMem,
!     512, 4*BLCKSZ/1024, INT_MAX},

      {"debug_level", PGC_USERSET, &DebugLvl,
      0, 0, 16},
***************
*** 413,420 ****


  /*
!  * Reset all options to their specified default values. Should only be
!  * called at program startup.
   */
  void
  ResetAllOptions(void)
--- 416,424 ----


  /*
!  * Reset all options to their specified default values.
!  *
!  * Default values come from: builtin, config file, command line.
   */
  void
  ResetAllOptions(void)
***************
*** 433,455 ****
      for (i = 0; ConfigureNamesString[i].name; i++)
      {
          char       *str = NULL;

!         if (!ConfigureNamesString[i].default_val
!                 && ConfigureNamesString[i].boot_default_val)
          {
!             str = strdup(ConfigureNamesString[i].boot_default_val);
              if (str == NULL)
                  elog(ERROR, "out of memory");

!             ConfigureNamesString[i].default_val = str;
          }
!         if (ConfigureNamesString[i].default_val)
!         {
!             str = strdup(ConfigureNamesString[i].default_val);
!             if (str == NULL)
!                 elog(ERROR, "out of memory");
!         }
!         *(ConfigureNamesString[i].variable) = str;
      }

      if (getenv("PGPORT"))
--- 437,459 ----
      for (i = 0; ConfigureNamesString[i].name; i++)
      {
          char       *str = NULL;
+         struct config_string *cf = &ConfigureNamesString[i];

!         if (!cf->default_val && cf->boot_default_val)
          {
!             str = strdup(cf->boot_default_val);
              if (str == NULL)
                  elog(ERROR, "out of memory");

!             cf->default_val = str;
          }
!
!         if (!cf->variable || !cf->default_val)
!             continue;
!
!         if (!*cf->variable || strcmp(cf->default_val, *cf->variable))
!             set_config_option_real(PGC_STRING, (struct config_generic *)cf,
!                     cf->default_val, true, false, ERROR);
      }

      if (getenv("PGPORT"))
***************
*** 607,612 ****
--- 611,617 ----
          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
***************
*** 644,649 ****
--- 649,663 ----
          elog(ERROR, "permission denied");


+     return set_config_option_real(type, (struct config_generic *)record,
+             value, DoIt, makeDefault, elevel);
+ }
+
+ static bool
+ set_config_option_real(enum config_type type, struct config_generic *record,
+                     const char *value, bool DoIt, bool makeDefault,
+                     int elevel)
+ {
      /*
       * Evaluate value and set variable
       */
***************
*** 659,665 ****

                      if (!parse_bool(value, &boolval))
                      {
!                         elog(elevel, "option '%s' requires a boolean value", name);
                          return false;
                      }
                      if (DoIt)
--- 673,680 ----

                      if (!parse_bool(value, &boolval))
                      {
!                         elog(elevel, "option '%s' requires a boolean value",
!                                 conf->name);
                          return false;
                      }
                      if (DoIt)
***************
*** 684,697 ****

                      if (!parse_int(value, &intval))
                      {
!                         elog(elevel, "option '%s' expects an integer value", name);
                          return false;
                      }
                      if (intval < conf->min || intval > conf->max)
                      {
                          elog(elevel, "option '%s' value %d is outside"
                               " of permissible range [%d .. %d]",
!                              name, intval, conf->min, conf->max);
                          return false;
                      }
                      if (DoIt)
--- 699,713 ----

                      if (!parse_int(value, &intval))
                      {
!                         elog(elevel, "option '%s' expects an integer value",
!                                 conf->name);
                          return false;
                      }
                      if (intval < conf->min || intval > conf->max)
                      {
                          elog(elevel, "option '%s' value %d is outside"
                               " of permissible range [%d .. %d]",
!                              conf->name, intval, conf->min, conf->max);
                          return false;
                      }
                      if (DoIt)
***************
*** 716,729 ****

                      if (!parse_real(value, &dval))
                      {
!                         elog(elevel, "option '%s' expects a real number", name);
                          return false;
                      }
                      if (dval < conf->min || dval > conf->max)
                      {
                          elog(elevel, "option '%s' value %g is outside"
                               " of permissible range [%g .. %g]",
!                              name, dval, conf->min, conf->max);
                          return false;
                      }
                      if (DoIt)
--- 732,746 ----

                      if (!parse_real(value, &dval))
                      {
!                         elog(elevel, "option '%s' expects a real number",
!                                 conf->name);
                          return false;
                      }
                      if (dval < conf->min || dval > conf->max)
                      {
                          elog(elevel, "option '%s' value %g is outside"
                               " of permissible range [%g .. %g]",
!                              conf->name, dval, conf->min, conf->max);
                          return false;
                      }
                      if (DoIt)
***************
*** 746,752 ****
                  {
                      if (conf->parse_hook && !(conf->parse_hook) (value))
                      {
!                         elog(elevel, "invalid value for option '%s': '%s'", name, value);
                          return false;
                      }
                      if (DoIt)
--- 763,769 ----
                  {
                      if (conf->parse_hook && !(conf->parse_hook) (value))
                      {
!                         elog(elevel, "invalid value for option '%s': '%s'", conf->name, value);
                          return false;
                      }
                      if (DoIt)

Re: reset all update

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

>
> * GUCify command line arguments.
> * ResetAllOptions() comment
> * split set_config_option()
> * use set_config_option_real() on string reset
>   - it calls hooks and free()'s the previous val
> * check if string val changed on reset
>
> --
> marko
>
>
> Index: src/backend/postmaster/postmaster.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
> retrieving revision 1.218
> diff -u -c -r1.218 postmaster.c
> *** src/backend/postmaster/postmaster.c    2001/06/11 04:12:29    1.218
> --- src/backend/postmaster/postmaster.c    2001/06/11 09:17:01
> ***************
> *** 422,435 ****
>   #ifndef USE_ASSERT_CHECKING
>                   postmaster_error("Assert checking is not compiled in.");
>   #else
> !                 assert_enabled = atoi(optarg);
>   #endif
>                   break;
>               case 'a':
>                   /* Can no longer set authentication method. */
>                   break;
>               case 'B':
> !                 NBuffers = atoi(optarg);
>                   break;
>               case 'b':
>                   /* Can no longer set the backend executable file to use. */
> --- 422,435 ----
>   #ifndef USE_ASSERT_CHECKING
>                   postmaster_error("Assert checking is not compiled in.");
>   #else
> !                 SetConfigOption("debug_assertions", optarg, PGC_POSTMASTER, true);
>   #endif
>                   break;
>               case 'a':
>                   /* Can no longer set authentication method. */
>                   break;
>               case 'B':
> !                 SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'b':
>                   /* Can no longer set the backend executable file to use. */
> ***************
> *** 443,465 ****
>                    * Turn on debugging for the postmaster and the backend
>                    * servers descended from it.
>                    */
> !                 DebugLvl = atoi(optarg);
>                   break;
>               case 'F':
> !                 enableFsync = false;
>                   break;
>               case 'h':
> !                 VirtualHost = optarg;
>                   break;
>               case 'i':
> !                 NetServer = true;
>                   break;
>               case 'k':
> !                 UnixSocketDir = optarg;
>                   break;
>   #ifdef USE_SSL
>               case 'l':
> !                 EnableSSL = true;
>                   break;
>   #endif
>               case 'm':
> --- 443,465 ----
>                    * Turn on debugging for the postmaster and the backend
>                    * servers descended from it.
>                    */
> !                 SetConfigOption("debug_level", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'F':
> !                 SetConfigOption("enable_fsync", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'h':
> !                 SetConfigOption("virtual_host", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'i':
> !                 SetConfigOption("tcpip_socket", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'k':
> !                 SetConfigOption("unix_socket_directory", optarg, PGC_POSTMASTER, true);
>                   break;
>   #ifdef USE_SSL
>               case 'l':
> !                 SetConfigOption("ssl", optarg, PGC_POSTMASTER, true);
>                   break;
>   #endif
>               case 'm':
> ***************
> *** 479,489 ****
>                    * The max number of backends to start. Can't set to less
>                    * than 1 or more than compiled-in limit.
>                    */
> !                 MaxBackends = atoi(optarg);
> !                 if (MaxBackends < 1)
> !                     MaxBackends = 1;
> !                 if (MaxBackends > MAXBACKENDS)
> !                     MaxBackends = MAXBACKENDS;
>                   break;
>               case 'n':
>                   /* Don't reinit shared mem after abnormal exit */
> --- 479,485 ----
>                    * The max number of backends to start. Can't set to less
>                    * than 1 or more than compiled-in limit.
>                    */
> !                 SetConfigOption("max_connections", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'n':
>                   /* Don't reinit shared mem after abnormal exit */
> ***************
> *** 500,506 ****
>                   strcpy(original_extraoptions, optarg);
>                   break;
>               case 'p':
> !                 PostPortNumber = atoi(optarg);
>                   break;
>               case 'S':
>
> --- 496,502 ----
>                   strcpy(original_extraoptions, optarg);
>                   break;
>               case 'p':
> !                 SetConfigOption("port", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 'S':
>
> ***************
> *** 510,516 ****
>                    * it's most badly needed on SysV-derived systems like
>                    * SVR4 and HP-UX.
>                    */
> !                 SilentMode = true;
>                   break;
>               case 's':
>
> --- 506,512 ----
>                    * it's most badly needed on SysV-derived systems like
>                    * SVR4 and HP-UX.
>                    */
> !                 SetConfigOption("silent_mode", optarg, PGC_POSTMASTER, true);
>                   break;
>               case 's':
>
> Index: src/backend/tcop/postgres.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/tcop/postgres.c,v
> retrieving revision 1.219
> diff -u -c -r1.219 postgres.c
> *** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
> --- src/backend/tcop/postgres.c    2001/06/11 09:17:07
> ***************
> *** 1108,1113 ****
> --- 1108,1115 ----
>       const char *DBName = NULL;
>       bool        secure = true;
>       int            errs = 0;
> +     GucContext    ctx;
> +     char        *tmp;
>
>       int            firstchar;
>       StringInfo    parser_input;
> ***************
> *** 1117,1122 ****
> --- 1119,1126 ----
>
>       char       *potential_DataDir = NULL;
>
> +     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
> +
>       /*
>        * Catch standard options before doing much else.  This even works on
>        * systems without getopt_long.
> ***************
> *** 1188,1194 ****
>           {
>               case 'A':
>   #ifdef USE_ASSERT_CHECKING
> !                 assert_enabled = atoi(optarg);
>   #else
>                   fprintf(stderr, "Assert checking is not compiled in\n");
>   #endif
> --- 1192,1198 ----
>           {
>               case 'A':
>   #ifdef USE_ASSERT_CHECKING
> !                 SetConfigOption("debug_assertions", optarg, ctx, true);
>   #else
>                   fprintf(stderr, "Assert checking is not compiled in\n");
>   #endif
> ***************
> *** 1200,1206 ****
>                    * specify the size of buffer pool
>                    */
>                   if (secure)
> !                     NBuffers = atoi(optarg);
>                   break;
>
>               case 'C':
> --- 1204,1210 ----
>                    * specify the size of buffer pool
>                    */
>                   if (secure)
> !                     SetConfigOption("shared_buffers", optarg, ctx, true);
>                   break;
>
>               case 'C':
> ***************
> *** 1217,1233 ****
>                   break;
>
>               case 'd':            /* debug level */
> !                 DebugLvl = atoi(optarg);
>                   if (DebugLvl >= 1);
> !                 Log_connections = true;
>                   if (DebugLvl >= 2)
> !                     Debug_print_query = true;
>                   if (DebugLvl >= 3)
> !                     Debug_print_parse = true;
>                   if (DebugLvl >= 4)
> !                     Debug_print_plan = true;
>                   if (DebugLvl >= 5)
> !                     Debug_print_rewritten = true;
>                   break;
>
>               case 'E':
> --- 1221,1238 ----
>                   break;
>
>               case 'd':            /* debug level */
> !                 tmp = "true";
> !                 SetConfigOption("debug_level", optarg, ctx, true);
>                   if (DebugLvl >= 1);
> !                 SetConfigOption("log_connections", tmp, ctx, true);
>                   if (DebugLvl >= 2)
> !                     SetConfigOption("debug_print_query", tmp, ctx, true);
>                   if (DebugLvl >= 3)
> !                     SetConfigOption("debug_print_parse", tmp, ctx, true);
>                   if (DebugLvl >= 4)
> !                     SetConfigOption("debug_print_plan", tmp, ctx, true);
>                   if (DebugLvl >= 5)
> !                     SetConfigOption("debug_print_rewritten", tmp, ctx, true);
>                   break;
>
>               case 'E':
> ***************
> *** 1252,1258 ****
>                    * turn off fsync
>                    */
>                   if (secure)
> !                     enableFsync = false;
>                   break;
>
>               case 'f':
> --- 1257,1263 ----
>                    * turn off fsync
>                    */
>                   if (secure)
> !                     SetConfigOption("fsync", "true", ctx, true);
>                   break;
>
>               case 'f':
> ***************
> *** 1260,1288 ****
>                   /*
>                    * f - forbid generation of certain plans
>                    */
>                   switch (optarg[0])
>                   {
>                       case 's':    /* seqscan */
> !                         enable_seqscan = false;
>                           break;
>                       case 'i':    /* indexscan */
> !                         enable_indexscan = false;
>                           break;
>                       case 't':    /* tidscan */
> !                         enable_tidscan = false;
>                           break;
>                       case 'n':    /* nestloop */
> !                         enable_nestloop = false;
>                           break;
>                       case 'm':    /* mergejoin */
> !                         enable_mergejoin = false;
>                           break;
>                       case 'h':    /* hashjoin */
> !                         enable_hashjoin = false;
>                           break;
>                       default:
>                           errs++;
>                   }
>                   break;
>
>               case 'i':
> --- 1265,1296 ----
>                   /*
>                    * f - forbid generation of certain plans
>                    */
> +                 tmp = NULL;
>                   switch (optarg[0])
>                   {
>                       case 's':    /* seqscan */
> !                         tmp = "enable_seqscan";
>                           break;
>                       case 'i':    /* indexscan */
> !                         tmp = "enable_indexscan";
>                           break;
>                       case 't':    /* tidscan */
> !                         tmp = "enable_tidscan";
>                           break;
>                       case 'n':    /* nestloop */
> !                         tmp = "enable_nestloop";
>                           break;
>                       case 'm':    /* mergejoin */
> !                         tmp = "enable_mergejoin";
>                           break;
>                       case 'h':    /* hashjoin */
> !                         tmp = "enable_hashjoin";
>                           break;
>                       default:
>                           errs++;
>                   }
> +                 if (tmp)
> +                     SetConfigOption(tmp, "false", ctx, true);
>                   break;
>
>               case 'i':
> ***************
> *** 1352,1364 ****
>                   /*
>                    * S - amount of sort memory to use in 1k bytes
>                    */
> !                 {
> !                     int            S;
> !
> !                     S = atoi(optarg);
> !                     if (S >= 4 * BLCKSZ / 1024)
> !                         SortMem = S;
> !                 }
>                   break;
>
>               case 's':
> --- 1360,1366 ----
>                   /*
>                    * S - amount of sort memory to use in 1k bytes
>                    */
> !                 SetConfigOption("sort_mem", optarg, ctx, true);
>                   break;
>
>               case 's':
> ***************
> *** 1366,1372 ****
>                   /*
>                    * s - report usage statistics (timings) after each query
>                    */
> !                 Show_query_stats = 1;
>                   break;
>
>               case 't':
> --- 1368,1374 ----
>                   /*
>                    * s - report usage statistics (timings) after each query
>                    */
> !                 SetConfigOption("show_query_stats", optarg, ctx, true);
>                   break;
>
>               case 't':
> ***************
> *** 1380,1402 ****
>                    *    caution: -s can not be used together with -t.
>                    * ----------------
>                    */
>                   switch (optarg[0])
>                   {
>                       case 'p':
>                           if (optarg[1] == 'a')
> !                             Show_parser_stats = 1;
>                           else if (optarg[1] == 'l')
> !                             Show_planner_stats = 1;
>                           else
>                               errs++;
>                           break;
>                       case 'e':
> !                         Show_executor_stats = 1;
>                           break;
>                       default:
>                           errs++;
>                           break;
>                   }
>                   break;
>
>               case 'v':
> --- 1382,1407 ----
>                    *    caution: -s can not be used together with -t.
>                    * ----------------
>                    */
> +                 tmp = NULL;
>                   switch (optarg[0])
>                   {
>                       case 'p':
>                           if (optarg[1] == 'a')
> !                             tmp = "show_parser_stats";
>                           else if (optarg[1] == 'l')
> !                             tmp = "show_planner_stats";
>                           else
>                               errs++;
>                           break;
>                       case 'e':
> !                         tmp = "show_parser_stats";
>                           break;
>                       default:
>                           errs++;
>                           break;
>                   }
> +                 if (tmp)
> +                     SetConfigOption(tmp, "true", ctx, true);
>                   break;
>
>               case 'v':
> Index: src/backend/utils/misc/guc.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
> retrieving revision 1.37
> diff -u -c -r1.37 guc.c
> *** src/backend/utils/misc/guc.c    2001/06/07 04:50:57    1.37
> --- src/backend/utils/misc/guc.c    2001/06/11 09:17:09
> ***************
> *** 143,148 ****
> --- 143,151 ----
>   };
>
>
> + static bool set_config_option_real(enum config_type type, struct config_generic *record,
> +                 const char *value, bool DoIt, bool makeDefault, int elevel);
> +
>   /*
>    * TO ADD AN OPTION:
>    *
> ***************
> *** 264,270 ****
>       DEF_PGPORT, 1, 65535},
>
>       {"sort_mem", PGC_USERSET, &SortMem,
> !     512, 1, INT_MAX},
>
>       {"debug_level", PGC_USERSET, &DebugLvl,
>       0, 0, 16},
> --- 267,273 ----
>       DEF_PGPORT, 1, 65535},
>
>       {"sort_mem", PGC_USERSET, &SortMem,
> !     512, 4*BLCKSZ/1024, INT_MAX},
>
>       {"debug_level", PGC_USERSET, &DebugLvl,
>       0, 0, 16},
> ***************
> *** 413,420 ****
>
>
>   /*
> !  * Reset all options to their specified default values. Should only be
> !  * called at program startup.
>    */
>   void
>   ResetAllOptions(void)
> --- 416,424 ----
>
>
>   /*
> !  * Reset all options to their specified default values.
> !  *
> !  * Default values come from: builtin, config file, command line.
>    */
>   void
>   ResetAllOptions(void)
> ***************
> *** 433,455 ****
>       for (i = 0; ConfigureNamesString[i].name; i++)
>       {
>           char       *str = NULL;
>
> !         if (!ConfigureNamesString[i].default_val
> !                 && ConfigureNamesString[i].boot_default_val)
>           {
> !             str = strdup(ConfigureNamesString[i].boot_default_val);
>               if (str == NULL)
>                   elog(ERROR, "out of memory");
>
> !             ConfigureNamesString[i].default_val = str;
>           }
> !         if (ConfigureNamesString[i].default_val)
> !         {
> !             str = strdup(ConfigureNamesString[i].default_val);
> !             if (str == NULL)
> !                 elog(ERROR, "out of memory");
> !         }
> !         *(ConfigureNamesString[i].variable) = str;
>       }
>
>       if (getenv("PGPORT"))
> --- 437,459 ----
>       for (i = 0; ConfigureNamesString[i].name; i++)
>       {
>           char       *str = NULL;
> +         struct config_string *cf = &ConfigureNamesString[i];
>
> !         if (!cf->default_val && cf->boot_default_val)
>           {
> !             str = strdup(cf->boot_default_val);
>               if (str == NULL)
>                   elog(ERROR, "out of memory");
>
> !             cf->default_val = str;
>           }
> !
> !         if (!cf->variable || !cf->default_val)
> !             continue;
> !
> !         if (!*cf->variable || strcmp(cf->default_val, *cf->variable))
> !             set_config_option_real(PGC_STRING, (struct config_generic *)cf,
> !                     cf->default_val, true, false, ERROR);
>       }
>
>       if (getenv("PGPORT"))
> ***************
> *** 607,612 ****
> --- 611,617 ----
>           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
> ***************
> *** 644,649 ****
> --- 649,663 ----
>           elog(ERROR, "permission denied");
>
>
> +     return set_config_option_real(type, (struct config_generic *)record,
> +             value, DoIt, makeDefault, elevel);
> + }
> +
> + static bool
> + set_config_option_real(enum config_type type, struct config_generic *record,
> +                     const char *value, bool DoIt, bool makeDefault,
> +                     int elevel)
> + {
>       /*
>        * Evaluate value and set variable
>        */
> ***************
> *** 659,665 ****
>
>                       if (!parse_bool(value, &boolval))
>                       {
> !                         elog(elevel, "option '%s' requires a boolean value", name);
>                           return false;
>                       }
>                       if (DoIt)
> --- 673,680 ----
>
>                       if (!parse_bool(value, &boolval))
>                       {
> !                         elog(elevel, "option '%s' requires a boolean value",
> !                                 conf->name);
>                           return false;
>                       }
>                       if (DoIt)
> ***************
> *** 684,697 ****
>
>                       if (!parse_int(value, &intval))
>                       {
> !                         elog(elevel, "option '%s' expects an integer value", name);
>                           return false;
>                       }
>                       if (intval < conf->min || intval > conf->max)
>                       {
>                           elog(elevel, "option '%s' value %d is outside"
>                                " of permissible range [%d .. %d]",
> !                              name, intval, conf->min, conf->max);
>                           return false;
>                       }
>                       if (DoIt)
> --- 699,713 ----
>
>                       if (!parse_int(value, &intval))
>                       {
> !                         elog(elevel, "option '%s' expects an integer value",
> !                                 conf->name);
>                           return false;
>                       }
>                       if (intval < conf->min || intval > conf->max)
>                       {
>                           elog(elevel, "option '%s' value %d is outside"
>                                " of permissible range [%d .. %d]",
> !                              conf->name, intval, conf->min, conf->max);
>                           return false;
>                       }
>                       if (DoIt)
> ***************
> *** 716,729 ****
>
>                       if (!parse_real(value, &dval))
>                       {
> !                         elog(elevel, "option '%s' expects a real number", name);
>                           return false;
>                       }
>                       if (dval < conf->min || dval > conf->max)
>                       {
>                           elog(elevel, "option '%s' value %g is outside"
>                                " of permissible range [%g .. %g]",
> !                              name, dval, conf->min, conf->max);
>                           return false;
>                       }
>                       if (DoIt)
> --- 732,746 ----
>
>                       if (!parse_real(value, &dval))
>                       {
> !                         elog(elevel, "option '%s' expects a real number",
> !                                 conf->name);
>                           return false;
>                       }
>                       if (dval < conf->min || dval > conf->max)
>                       {
>                           elog(elevel, "option '%s' value %g is outside"
>                                " of permissible range [%g .. %g]",
> !                              conf->name, dval, conf->min, conf->max);
>                           return false;
>                       }
>                       if (DoIt)
> ***************
> *** 746,752 ****
>                   {
>                       if (conf->parse_hook && !(conf->parse_hook) (value))
>                       {
> !                         elog(elevel, "invalid value for option '%s': '%s'", name, value);
>                           return false;
>                       }
>                       if (DoIt)
> --- 763,769 ----
>                   {
>                       if (conf->parse_hook && !(conf->parse_hook) (value))
>                       {
> !                         elog(elevel, "invalid value for option '%s': '%s'", conf->name, value);
>                           return false;
>                       }
>                       if (DoIt)
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: reset all update

From
Marko Kreen
Date:
On Tue, Jun 12, 2001 at 10:05:29PM +0200, Peter Eisentraut wrote:
> Marko Kreen writes:
>
> > On Tue, Jun 12, 2001 at 09:37:43PM +0200, Peter Eisentraut wrote:
> > > Marko Kreen writes:
> > > > *** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
> > > > --- src/backend/tcop/postgres.c    2001/06/11 09:17:07
> > > > +     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
> > > > +
> > >
> > > This is wrong.  If you're in PostgresMain then the context is PGC_BACKEND
> > > -- by definition.
> >
> > Well, but how do you explain this: (line 1463 in current CVS):
> >
> >          /* all options are allowed if not under postmaster */
> >          SetConfigOption(name, value,
> >             (IsUnderPostmaster) ? PGC_BACKEND : PGC_POSTMASTER, true);
> >
> > As I understand, when you run ./postgres directly, you get
> > PGC_POSTMASTER, which includes PGC_BACKEND.
>
> You're right.  Objection withdrawn.  ;-)

:)

The following should be applied on top of previous patch.
Moved the comment too.

--
marko


*** src/backend/tcop/postgres.c.orig    Tue Jun 12 21:53:26 2001
--- src/backend/tcop/postgres.c    Tue Jun 12 22:07:42 2001
***************
*** 1119,1124 ****
--- 1119,1125 ----

      char       *potential_DataDir = NULL;

+     /* all options are allowed if not under postmaster */
      ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;

      /*
***************
*** 1465,1473 ****
                              elog(ERROR, "-c %s requires argument", optarg);
                      }

!                     /* all options are allowed if not under postmaster */
!                     SetConfigOption(name, value,
!                      (IsUnderPostmaster) ? PGC_BACKEND : PGC_POSTMASTER, true);
                      free(name);
                      if (value)
                          free(value);
--- 1466,1472 ----
                              elog(ERROR, "-c %s requires argument", optarg);
                      }

!                     SetConfigOption(name, value, ctx, true);
                      free(name);
                      if (value)
                          free(value);
*** src/backend/utils/misc/guc.c.orig    Tue Jun 12 21:50:51 2001
--- src/backend/utils/misc/guc.c    Tue Jun 12 22:19:23 2001
***************
*** 448,457 ****
              cf->default_val = str;
          }

!         if (!cf->variable || !cf->default_val)
!             continue;
!
!         if (!*cf->variable || strcmp(cf->default_val, *cf->variable))
              set_config_option_real(PGC_STRING, (struct config_generic *)cf,
                      cf->default_val, true, false, ERROR);
      }
--- 448,454 ----
              cf->default_val = str;
          }

!         if (*cf->variable == NULL || strcmp(cf->default_val, *cf->variable) != 0)
              set_config_option_real(PGC_STRING, (struct config_generic *)cf,
                      cf->default_val, true, false, ERROR);
      }

Re: reset all update

From
Peter Eisentraut
Date:
Marko Kreen writes:

> On Tue, Jun 12, 2001 at 09:37:43PM +0200, Peter Eisentraut wrote:
> > Marko Kreen writes:
> > > *** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
> > > --- src/backend/tcop/postgres.c    2001/06/11 09:17:07
> > > +     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
> > > +
> >
> > This is wrong.  If you're in PostgresMain then the context is PGC_BACKEND
> > -- by definition.
>
> Well, but how do you explain this: (line 1463 in current CVS):
>
>          /* all options are allowed if not under postmaster */
>          SetConfigOption(name, value,
>             (IsUnderPostmaster) ? PGC_BACKEND : PGC_POSTMASTER, true);
>
> As I understand, when you run ./postgres directly, you get
> PGC_POSTMASTER, which includes PGC_BACKEND.

You're right.  Objection withdrawn.  ;-)

--
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Re: reset all update

From
Peter Eisentraut
Date:
Marko Kreen writes:

> * GUCify command line arguments.
> * ResetAllOptions() comment
> * split set_config_option()
> * use set_config_option_real() on string reset
>   - it calls hooks and free()'s the previous val
> * check if string val changed on reset

Seems on track.  Comments below.


> Index: src/backend/tcop/postgres.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/tcop/postgres.c,v
> retrieving revision 1.219
> diff -u -c -r1.219 postgres.c
> *** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
> --- src/backend/tcop/postgres.c    2001/06/11 09:17:07
> ***************
> *** 1108,1113 ****
> --- 1108,1115 ----
>       const char *DBName = NULL;
>       bool        secure = true;
>       int            errs = 0;
> +     GucContext    ctx;
> +     char        *tmp;
>
>       int            firstchar;
>       StringInfo    parser_input;
> ***************
> *** 1117,1122 ****
> --- 1119,1126 ----
>
>       char       *potential_DataDir = NULL;
>
> +     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
> +

This is wrong.  If you're in PostgresMain then the context is PGC_BACKEND
-- by definition.

>       /*
>        * Catch standard options before doing much else.  This even works on
>        * systems without getopt_long.
> ***************

> Index: src/backend/utils/misc/guc.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
> retrieving revision 1.37
> diff -u -c -r1.37 guc.c
> *** src/backend/utils/misc/guc.c    2001/06/07 04:50:57    1.37
> --- src/backend/utils/misc/guc.c    2001/06/11 09:17:09
> ***************

> --- 437,459 ----
>       for (i = 0; ConfigureNamesString[i].name; i++)
>       {
>           char       *str = NULL;
> +         struct config_string *cf = &ConfigureNamesString[i];
>
> !         if (!cf->default_val && cf->boot_default_val)
>           {
> !             str = strdup(cf->boot_default_val);
>               if (str == NULL)
>                   elog(ERROR, "out of memory");
>
> !             cf->default_val = str;
>           }
> !
> !         if (!cf->variable || !cf->default_val)
> !             continue;

This shouldn't happen.  Just let it crash.

> !
> !         if (!*cf->variable || strcmp(cf->default_val, *cf->variable))

Evil coding style alert.  Suggest:

    if (*cf->variable == NULL || strcmp(cf->default_val, *cf->variable) != 0)

> !             set_config_option_real(PGC_STRING, (struct config_generic *)cf,
> !                     cf->default_val, true, false, ERROR);
>       }
>
>       if (getenv("PGPORT"))
> ***************

--
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter


Re: reset all update

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> On Tue, Jun 12, 2001 at 10:05:29PM +0200, Peter Eisentraut wrote:
> > Marko Kreen writes:
> >
> > > On Tue, Jun 12, 2001 at 09:37:43PM +0200, Peter Eisentraut wrote:
> > > > Marko Kreen writes:
> > > > > *** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
> > > > > --- src/backend/tcop/postgres.c    2001/06/11 09:17:07
> > > > > +     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
> > > > > +
> > > >
> > > > This is wrong.  If you're in PostgresMain then the context is PGC_BACKEND
> > > > -- by definition.
> > >
> > > Well, but how do you explain this: (line 1463 in current CVS):
> > >
> > >          /* all options are allowed if not under postmaster */
> > >          SetConfigOption(name, value,
> > >             (IsUnderPostmaster) ? PGC_BACKEND : PGC_POSTMASTER, true);
> > >
> > > As I understand, when you run ./postgres directly, you get
> > > PGC_POSTMASTER, which includes PGC_BACKEND.
> >
> > You're right.  Objection withdrawn.  ;-)
>
> :)
>
> The following should be applied on top of previous patch.
> Moved the comment too.
>
> --
> marko
>
>
> *** src/backend/tcop/postgres.c.orig    Tue Jun 12 21:53:26 2001
> --- src/backend/tcop/postgres.c    Tue Jun 12 22:07:42 2001
> ***************
> *** 1119,1124 ****
> --- 1119,1125 ----
>
>       char       *potential_DataDir = NULL;
>
> +     /* all options are allowed if not under postmaster */
>       ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
>
>       /*
> ***************
> *** 1465,1473 ****
>                               elog(ERROR, "-c %s requires argument", optarg);
>                       }
>
> !                     /* all options are allowed if not under postmaster */
> !                     SetConfigOption(name, value,
> !                      (IsUnderPostmaster) ? PGC_BACKEND : PGC_POSTMASTER, true);
>                       free(name);
>                       if (value)
>                           free(value);
> --- 1466,1472 ----
>                               elog(ERROR, "-c %s requires argument", optarg);
>                       }
>
> !                     SetConfigOption(name, value, ctx, true);
>                       free(name);
>                       if (value)
>                           free(value);
> *** src/backend/utils/misc/guc.c.orig    Tue Jun 12 21:50:51 2001
> --- src/backend/utils/misc/guc.c    Tue Jun 12 22:19:23 2001
> ***************
> *** 448,457 ****
>               cf->default_val = str;
>           }
>
> !         if (!cf->variable || !cf->default_val)
> !             continue;
> !
> !         if (!*cf->variable || strcmp(cf->default_val, *cf->variable))
>               set_config_option_real(PGC_STRING, (struct config_generic *)cf,
>                       cf->default_val, true, false, ERROR);
>       }
> --- 448,454 ----
>               cf->default_val = str;
>           }
>
> !         if (*cf->variable == NULL || strcmp(cf->default_val, *cf->variable) != 0)
>               set_config_option_real(PGC_STRING, (struct config_generic *)cf,
>                       cf->default_val, true, false, ERROR);
>       }
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: reset all update

From
Marko Kreen
Date:
On Tue, Jun 12, 2001 at 09:37:43PM +0200, Peter Eisentraut wrote:
> Marko Kreen writes:
> > *** src/backend/tcop/postgres.c    2001/06/07 04:50:57    1.219
> > --- src/backend/tcop/postgres.c    2001/06/11 09:17:07
> > +     ctx = IsUnderPostmaster ? PGC_BACKEND : PGC_POSTMASTER;
> > +
>
> This is wrong.  If you're in PostgresMain then the context is PGC_BACKEND
> -- by definition.

Well, but how do you explain this: (line 1463 in current CVS):

         /* all options are allowed if not under postmaster */
         SetConfigOption(name, value,
            (IsUnderPostmaster) ? PGC_BACKEND : PGC_POSTMASTER, true);

As I understand, when you run ./postgres directly, you get
PGC_POSTMASTER, which includes PGC_BACKEND.

And sorry, I should have updated that check to 'ctx' too, it
would have been clearer.

> > !         if (!cf->variable || !cf->default_val)
> > !             continue;
>
> This shouldn't happen.  Just let it crash.
>
> > !
> > !         if (!*cf->variable || strcmp(cf->default_val, *cf->variable))
>
> Evil coding style alert.  Suggest:

Ok, will send a update to that, waiting first your
comment on the ctx issue.


--
marko


Re: reset all update

From
Tom Lane
Date:
Marko Kreen <marko@l-t.ee> writes:
> [ reset all patch ]

Hmm, you seem to have overlapped with some changes I was just making.
The original version of your patch still hasn't arrived here :-(
so I don't know what you intended to do.  But you might want to check
what I just committed.

            regards, tom lane

Re: reset all update

From
Marko Kreen
Date:
On Tue, Jun 12, 2001 at 07:13:31PM -0400, Tom Lane wrote:
> Marko Kreen <marko@l-t.ee> writes:
> > [ reset all patch ]
>
> Hmm, you seem to have overlapped with some changes I was just making.
> The original version of your patch still hasn't arrived here :-(
> so I don't know what you intended to do.  But you might want to check
> what I just committed.

My idea was that RESET should not check permissions, all perm
check are in set_*.  vars that are not SET-able by user, will
have *variable == default_val, so reset can safely go through
all of them, and not mess anything up.  But this means the
default_val must be 'right'.  Therefore I made also cmdline args go
through GUC, so that they get right 'defaults'.  As an added
bonus, this means that all var range checks are in one place.

Now I do not know what to do.  How to handle permissions is
your call.  I still think that GUCifying cmdline is Good, because
of the var checks.  Also, when in future there will be 'set
hooks' which eg. (re)allocate memory, it is good when all var
changes go through one place.  Should I resubmit the patch, now
against your changes?  Do you still think perm check in reset is
necessary?

--
marko


Re: reset all update

From
Tom Lane
Date:
Marko Kreen <marko@l-t.ee> writes:
> My idea was that RESET should not check permissions, all perm
> check are in set_*.  vars that are not SET-able by user, will
> have *variable == default_val, so reset can safely go through
> all of them, and not mess anything up.  But this means the
> default_val must be 'right'.

Indeed.  My feeling is the opposite: there is no scenario in which RESET
ALL should be changing variables that are POSTMASTER, BACKEND, or SIGHUP
level; therefore it's best that it not even try.  Belt-and-suspenders
programming, if you like, since I also agree that the default should be
correct.

> I still think that GUCifying cmdline is Good, because
> of the var checks.  Also, when in future there will be 'set
> hooks' which eg. (re)allocate memory, it is good when all var
> changes go through one place.

Yes, I agree with both these points.  I think you are right that
replacing all the direct assignments with GUC calls is a good idea.

I apologize for not having noticed your patch before I committed
what I was doing; the conflict is my fault.  If you have time to redo
your changes atop mine, it would be appreciated.  Otherwise I'll take
responsibility for cleaning up the mess.

BTW, I would recommend that you not add logic to suppress the assignment
when the value is not changing.  Now that there are assign_hooks for all
variable types, it seems to me that it is the assign_hook's
responsibility to decide whether it wants to optimize that case or not.

            regards, tom lane

Re: reset all update

From
Bruce Momjian
Date:
> I apologize for not having noticed your patch before I committed
> what I was doing; the conflict is my fault.  If you have time to redo
> your changes atop mine, it would be appreciated.  Otherwise I'll take
> responsibility for cleaning up the mess.
>
> BTW, I would recommend that you not add logic to suppress the assignment
> when the value is not changing.  Now that there are assign_hooks for all
> variable types, it seems to me that it is the assign_hook's
> responsibility to decide whether it wants to optimize that case or not.

This assume the assign hooks have no negative effects.  Should we call
them if the value isn't changed?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: reset all update

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> BTW, I would recommend that you not add logic to suppress the assignment
>> when the value is not changing.  Now that there are assign_hooks for all
>> variable types, it seems to me that it is the assign_hook's
>> responsibility to decide whether it wants to optimize that case or not.

> This assume the assign hooks have no negative effects.  Should we call
> them if the value isn't changed?

It's the hook's responsibility not to do anything that's incorrect.

Consider the other case: maybe the hook *needs* to do something even
though the value is not really changing.  I haven't got a good example
offhand, but in cache-flush tasks that's not impossible.  Or perhaps
more plausible, there may be state not directly visible to GUC that
needs to be checked, even when the displayable var itself is correct
already.

Bottom line: I'd rather require the hooks to be slightly intelligent
than cripple them with a GUC-knows-better-than-they-do policy.  The
point of the hooks is to know things that the core GUC code doesn't,
after all.

            regards, tom lane

Re: reset all update

From
Marko Kreen
Date:
On Wed, Jun 13, 2001 at 10:39:21AM -0400, Tom Lane wrote:
> I apologize for not having noticed your patch before I committed
> what I was doing; the conflict is my fault.  If you have time to redo
> your changes atop mine, it would be appreciated.  Otherwise I'll take
> responsibility for cleaning up the mess.

I'll look it over.

--
marko