Thread: reset all update
* 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)
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
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); }
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
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
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
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
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
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
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
> 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
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
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