[gsmith@gregsmith.com: Re: [patch] GUC source file and line number]
Greg just sent me this patch, augmenting the one I sent to add source
file and line to GUC vars; Greg's patch adds a column with the default
value of each var.
I forward it to -hackers to have a public Message-Id to link to in the
Commitfest page.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
guc-with-defaults.patchtext/plain; charset=US-ASCII; name=guc-with-defaults.patchDownload
Index: doc/src/sgml/catalogs.sgml
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/doc/src/sgml/catalogs.sgml,v
retrieving revision 2.172
diff -c -r2.172 catalogs.sgml
*** doc/src/sgml/catalogs.sgml 30 Jul 2008 17:05:04 -0000 2.172
--- doc/src/sgml/catalogs.sgml 1 Sep 2008 23:55:25 -0000
***************
*** 6414,6419 ****
--- 6414,6436 ----
<entry>Allowed values in enum parameters (NULL for non-enum
values)</entry>
</row>
+ <row>
+ <entry><structfield>default_val</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>Default value if the parameter is not explicitly set</entry>
+ </row>
+ <row>
+ <entry><structfield>sourcefile</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>Input file the current value was set from (if any), helpful
+ when using configuration include directives</entry>
+ </row>
+ <row>
+ <entry><structfield>sourceline</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>Line number within the sourcefile the current value was set
+ from</entry>
+ </row>
</tbody>
</tgroup>
</table>
Index: src/backend/utils/adt/ri_triggers.c
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/backend/utils/adt/ri_triggers.c,v
retrieving revision 1.109
diff -c -r1.109 ri_triggers.c
*** src/backend/utils/adt/ri_triggers.c 19 May 2008 04:14:24 -0000 1.109
--- src/backend/utils/adt/ri_triggers.c 1 Sep 2008 23:27:02 -0000
***************
*** 2738,2743 ****
--- 2738,2744 ----
snprintf(workmembuf, sizeof(workmembuf), "%d", maintenance_work_mem);
(void) set_config_option("work_mem", workmembuf,
PGC_USERSET, PGC_S_SESSION,
+ NULL, 0,
GUC_ACTION_LOCAL, true);
if (SPI_connect() != SPI_OK_CONNECT)
***************
*** 2827,2832 ****
--- 2828,2834 ----
snprintf(workmembuf, sizeof(workmembuf), "%d", old_work_mem);
(void) set_config_option("work_mem", workmembuf,
PGC_USERSET, PGC_S_SESSION,
+ NULL, 0,
GUC_ACTION_LOCAL, true);
return true;
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.56
diff -c -r1.56 guc-file.l
*** src/backend/utils/misc/guc-file.l 22 Aug 2008 00:20:40 -0000 1.56
--- src/backend/utils/misc/guc-file.l 1 Sep 2008 23:27:02 -0000
***************
*** 39,44 ****
--- 39,46 ----
{
char *name;
char *value;
+ char *filename;
+ int sourceline;
struct name_value_pair *next;
};
***************
*** 232,238 ****
}
if (!set_config_option(item->name, item->value, context,
! PGC_S_FILE, GUC_ACTION_SET, false))
goto cleanup_list;
}
--- 234,240 ----
}
if (!set_config_option(item->name, item->value, context,
! PGC_S_FILE, NULL, 0, GUC_ACTION_SET, false))
goto cleanup_list;
}
***************
*** 280,286 ****
/* Now we can re-apply the wired-in default */
set_config_option(gconf->name, NULL, context, PGC_S_DEFAULT,
! GUC_ACTION_SET, true);
}
/*
--- 282,288 ----
/* Now we can re-apply the wired-in default */
set_config_option(gconf->name, NULL, context, PGC_S_DEFAULT,
! NULL, 0, GUC_ACTION_SET, true);
}
/*
***************
*** 296,314 ****
envvar = getenv("PGDATESTYLE");
if (envvar != NULL)
set_config_option("datestyle", envvar, PGC_POSTMASTER,
! PGC_S_ENV_VAR, GUC_ACTION_SET, true);
envvar = getenv("PGCLIENTENCODING");
if (envvar != NULL)
set_config_option("client_encoding", envvar, PGC_POSTMASTER,
! PGC_S_ENV_VAR, GUC_ACTION_SET, true);
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
set_config_option(item->name, item->value, context,
! PGC_S_FILE, GUC_ACTION_SET, true);
}
/* Remember when we last successfully loaded the config file. */
--- 298,317 ----
envvar = getenv("PGDATESTYLE");
if (envvar != NULL)
set_config_option("datestyle", envvar, PGC_POSTMASTER,
! PGC_S_ENV_VAR, NULL, 0, GUC_ACTION_SET, true);
envvar = getenv("PGCLIENTENCODING");
if (envvar != NULL)
set_config_option("client_encoding", envvar, PGC_POSTMASTER,
! PGC_S_ENV_VAR, NULL, 0, GUC_ACTION_SET, true);
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
set_config_option(item->name, item->value, context,
! PGC_S_FILE, item->filename, item->sourceline,
! GUC_ACTION_SET, true);
}
/* Remember when we last successfully loaded the config file. */
***************
*** 483,488 ****
--- 486,493 ----
pfree(item->value);
item->name = opt_name;
item->value = opt_value;
+ item->filename = pstrdup(config_file);
+ item->sourceline = ConfigFileLineno-1;
}
else
{
***************
*** 490,495 ****
--- 495,502 ----
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
+ item->filename = pstrdup(config_file);
+ item->sourceline = ConfigFileLineno-1;
item->next = *head_p;
*head_p = item;
if (*tail_p == NULL)
***************
*** 502,507 ****
--- 509,516 ----
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
+ item->filename = pstrdup(config_file);
+ item->sourceline = ConfigFileLineno-1;
item->next = NULL;
if (*head_p == NULL)
*head_p = item;
***************
*** 553,558 ****
--- 562,568 ----
pfree(item->name);
pfree(item->value);
+ pfree(item->filename);
pfree(item);
item = next;
}
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.470
diff -c -r1.470 guc.c
*** src/backend/utils/misc/guc.c 25 Aug 2008 15:11:00 -0000 1.470
--- src/backend/utils/misc/guc.c 1 Sep 2008 23:37:09 -0000
***************
*** 3200,3205 ****
--- 3200,3209 ----
gconf->reset_source = PGC_S_DEFAULT;
gconf->source = PGC_S_DEFAULT;
gconf->stack = NULL;
+ gconf->sourcefile = NULL;
+ gconf->sourceline = 0;
+ gconf->reset_sourcefile = NULL;
+ gconf->reset_sourceline = 0;
switch (gconf->vartype)
{
***************
*** 3541,3547 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_INT:
--- 3545,3550 ----
***************
*** 3553,3559 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_REAL:
--- 3556,3561 ----
***************
*** 3565,3571 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_STRING:
--- 3567,3572 ----
***************
*** 3594,3600 ****
}
set_string_field(conf, conf->variable, str);
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_ENUM:
--- 3595,3600 ----
***************
*** 3606,3616 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
}
if (gconf->flags & GUC_REPORT)
ReportGUCOption(gconf);
}
--- 3606,3621 ----
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
break;
}
}
+ gconf->source = gconf->reset_source;
+ if (gconf->sourcefile)
+ free(gconf->sourcefile);
+ gconf->sourcefile = gconf->reset_sourcefile;
+ gconf->sourceline = gconf->reset_sourceline;
+
if (gconf->flags & GUC_REPORT)
ReportGUCOption(gconf);
}
***************
*** 4499,4504 ****
--- 4504,4534 ----
return result;
}
+ /*
+ * Set the fields for source file and line number the setting came from.
+ * If the setting came from something that's not a file, clear the fields.
+ */
+ static void
+ set_config_sourcefile(struct config_generic *conf, const char *sourcefile,
+ int sourceline)
+ {
+ if (conf->source != PGC_S_FILE)
+ {
+ if (conf->sourcefile)
+ free(conf->sourcefile);
+ conf->sourcefile = NULL;
+ conf->sourceline = 0;
+ }
+ else
+ {
+ /* NULL sourcefile means don't touch what's in the field */
+ if (sourcefile == NULL)
+ return;
+
+ conf->sourcefile = guc_strdup(ERROR, sourcefile);
+ conf->sourceline = sourceline;
+ }
+ }
/*
* Sets option `name' to given value. The value should be a string
***************
*** 4530,4535 ****
--- 4560,4566 ----
bool
set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
+ const char *sourcefile, int sourceline,
GucAction action, bool changeVal)
{
struct config_generic *record;
***************
*** 4722,4727 ****
--- 4753,4760 ----
{
newval = conf->reset_val;
source = conf->gen.reset_source;
+ sourcefile = conf->gen.reset_sourcefile;
+ sourceline = conf->gen.reset_sourceline;
}
/* Save old value to support transaction abort */
***************
*** 4742,4747 ****
--- 4775,4781 ----
{
*conf->variable = newval;
conf->gen.source = source;
+ set_config_sourcefile(&conf->gen, sourcefile, sourceline);
}
if (makeDefault)
{
***************
*** 4751,4756 ****
--- 4785,4794 ----
{
conf->reset_val = newval;
conf->gen.reset_source = source;
+ if (conf->gen.reset_sourcefile)
+ free(conf->gen.reset_sourcefile);
+ conf->gen.reset_sourcefile = sourcefile ? guc_strdup(elevel, sourcefile) : NULL;
+ conf->gen.reset_sourceline = sourceline;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
***************
*** 4797,4802 ****
--- 4835,4842 ----
{
newval = conf->reset_val;
source = conf->gen.reset_source;
+ sourcefile = conf->gen.reset_sourcefile;
+ sourceline = conf->gen.reset_sourceline;
}
/* Save old value to support transaction abort */
***************
*** 4817,4822 ****
--- 4857,4863 ----
{
*conf->variable = newval;
conf->gen.source = source;
+ set_config_sourcefile(&conf->gen, sourcefile, sourceline);
}
if (makeDefault)
{
***************
*** 4826,4831 ****
--- 4867,4876 ----
{
conf->reset_val = newval;
conf->gen.reset_source = source;
+ if (conf->gen.reset_sourcefile)
+ free(conf->gen.reset_sourcefile);
+ conf->gen.reset_sourcefile = sourcefile ? guc_strdup(elevel, sourcefile) : NULL;
+ conf->gen.reset_sourceline = sourceline;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
***************
*** 4869,4874 ****
--- 4914,4921 ----
{
newval = conf->reset_val;
source = conf->gen.reset_source;
+ sourcefile = conf->gen.reset_sourcefile;
+ sourceline = conf->gen.reset_sourceline;
}
/* Save old value to support transaction abort */
***************
*** 4889,4894 ****
--- 4936,4942 ----
{
*conf->variable = newval;
conf->gen.source = source;
+ set_config_sourcefile(&conf->gen, sourcefile, sourceline);
}
if (makeDefault)
{
***************
*** 4898,4903 ****
--- 4946,4955 ----
{
conf->reset_val = newval;
conf->gen.reset_source = source;
+ if (conf->gen.reset_sourcefile)
+ free(conf->gen.reset_sourcefile);
+ conf->gen.reset_sourcefile = sourcefile ? guc_strdup(elevel, sourcefile) : NULL;
+ conf->gen.reset_sourceline = sourceline;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
***************
*** 4956,4961 ****
--- 5008,5015 ----
return false;
}
source = conf->gen.reset_source;
+ sourcefile = conf->gen.reset_sourcefile;
+ sourceline = conf->gen.reset_sourceline;
}
/* Save old value to support transaction abort */
***************
*** 5003,5008 ****
--- 5057,5063 ----
{
set_string_field(conf, conf->variable, newval);
conf->gen.source = source;
+ set_config_sourcefile(&conf->gen, sourcefile, sourceline);
}
if (makeDefault)
{
***************
*** 5012,5017 ****
--- 5067,5076 ----
{
set_string_field(conf, &conf->reset_val, newval);
conf->gen.reset_source = source;
+ if (conf->gen.reset_sourcefile)
+ free(conf->gen.reset_sourcefile);
+ conf->gen.reset_sourcefile = sourcefile ? guc_strdup(elevel, sourcefile) : NULL;
+ conf->gen.reset_sourceline = sourceline;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
***************
*** 5056,5061 ****
--- 5115,5122 ----
{
newval = conf->reset_val;
source = conf->gen.reset_source;
+ sourcefile = conf->gen.reset_sourcefile;
+ sourceline = conf->gen.reset_sourceline;
}
/* Save old value to support transaction abort */
***************
*** 5077,5082 ****
--- 5138,5144 ----
{
*conf->variable = newval;
conf->gen.source = source;
+ set_config_sourcefile(&conf->gen, sourcefile, sourceline);
}
if (makeDefault)
{
***************
*** 5086,5091 ****
--- 5148,5157 ----
{
conf->reset_val = newval;
conf->gen.reset_source = source;
+ if (conf->gen.reset_sourcefile)
+ free(conf->gen.reset_sourcefile);
+ conf->gen.reset_sourcefile = sourcefile ? guc_strdup(elevel, sourcefile) : NULL;
+ conf->gen.reset_sourceline = sourceline;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
***************
*** 5111,5127 ****
* Set a config option to the given value. See also set_config_option,
* this is just the wrapper to be called from outside GUC. NB: this
* is used only for non-transactional operations.
*/
void
SetConfigOption(const char *name, const char *value,
GucContext context, GucSource source)
{
! (void) set_config_option(name, value, context, source,
GUC_ACTION_SET, true);
}
-
/*
* Fetch the current value of the option `name'. If the option doesn't exist,
* throw an ereport and don't return.
--- 5177,5195 ----
* Set a config option to the given value. See also set_config_option,
* this is just the wrapper to be called from outside GUC. NB: this
* is used only for non-transactional operations.
+ *
+ * Note: there is no support here for setting source file/line, as it
+ * is currently not needed.
*/
void
SetConfigOption(const char *name, const char *value,
GucContext context, GucSource source)
{
! (void) set_config_option(name, value, context, source, NULL, 0,
GUC_ACTION_SET, true);
}
/*
* Fetch the current value of the option `name'. If the option doesn't exist,
* throw an ereport and don't return.
***************
*** 5424,5429 ****
--- 5492,5498 ----
ExtractSetVariableArgs(stmt),
(superuser() ? PGC_SUSET : PGC_USERSET),
PGC_S_SESSION,
+ NULL, 0,
action,
true);
break;
***************
*** 5481,5486 ****
--- 5550,5556 ----
NULL,
(superuser() ? PGC_SUSET : PGC_USERSET),
PGC_S_SESSION,
+ NULL, 0,
action,
true);
break;
***************
*** 5526,5531 ****
--- 5596,5602 ----
argstring,
(superuser() ? PGC_SUSET : PGC_USERSET),
PGC_S_SESSION,
+ NULL, 0,
is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET,
true);
}
***************
*** 5569,5574 ****
--- 5640,5646 ----
value,
(superuser() ? PGC_SUSET : PGC_USERSET),
PGC_S_SESSION,
+ NULL, 0,
is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET,
true);
***************
*** 5661,5666 ****
--- 5733,5739 ----
if (value)
set_config_option(name, value,
pHolder->gen.context, pHolder->gen.source,
+ NULL, 0,
GUC_ACTION_SET, true);
/*
***************
*** 6056,6061 ****
--- 6129,6136 ----
{
case PGC_BOOL:
{
+ struct config_bool *lconf = (struct config_bool *) conf;
+
/* min_val */
values[9] = NULL;
***************
*** 6064,6069 ****
--- 6139,6148 ----
/* enumvals */
values[11] = NULL;
+
+ /* default_val */
+ snprintf(buffer, sizeof(buffer), "%s", lconf->boot_val ? "on" : "off");
+ values[12] = pstrdup(buffer);
}
break;
***************
*** 6081,6086 ****
--- 6160,6169 ----
/* enumvals */
values[11] = NULL;
+
+ /* default_val */
+ snprintf(buffer, sizeof(buffer), "%d", lconf->boot_val);
+ values[12] = pstrdup(buffer);
}
break;
***************
*** 6098,6108 ****
--- 6181,6197 ----
/* enumvals */
values[11] = NULL;
+
+ /* default_val */
+ snprintf(buffer, sizeof(buffer), "%g", lconf->boot_val);
+ values[12] = pstrdup(buffer);
}
break;
case PGC_STRING:
{
+ struct config_string *lconf = (struct config_string *) conf;
+
/* min_val */
values[9] = NULL;
***************
*** 6111,6121 ****
--- 6200,6222 ----
/* enumvals */
values[11] = NULL;
+
+ /* default_val */
+ if (lconf->boot_val == NULL)
+ {
+ values[12] = NULL;
+ } else
+ {
+ snprintf(buffer, sizeof(buffer), "%s", lconf->boot_val);
+ values[12] = pstrdup(buffer);
+ }
}
break;
case PGC_ENUM:
{
+ struct config_enum *lconf = (struct config_enum *) conf;
+
/* min_val */
values[9] = NULL;
***************
*** 6124,6129 ****
--- 6225,6234 ----
/* enumvals */
values[11] = config_enum_get_options((struct config_enum *) conf, "", "");
+
+ /* default_val */
+ snprintf(buffer, sizeof(buffer), "%s", config_enum_lookup_by_value(lconf, lconf->boot_val));
+ values[12] = pstrdup(buffer);
}
break;
***************
*** 6141,6149 ****
--- 6246,6270 ----
/* enumvals */
values[11] = NULL;
+
+ /* default_val */
+ values[12] = NULL;
}
break;
}
+
+ /* If the setting came from a config file, set the source location */
+ if (conf->source == PGC_S_FILE)
+ {
+ values[13] = conf->sourcefile;
+ snprintf(buffer, sizeof(buffer), "%d", conf->sourceline);
+ values[14] = pstrdup(buffer);
+ }
+ else
+ {
+ values[13] = NULL;
+ values[14] = NULL;
+ }
}
/*
***************
*** 6179,6185 ****
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
! #define NUM_PG_SETTINGS_ATTS 12
Datum
show_all_settings(PG_FUNCTION_ARGS)
--- 6300,6306 ----
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
! #define NUM_PG_SETTINGS_ATTS 15
Datum
show_all_settings(PG_FUNCTION_ARGS)
***************
*** 6231,6236 ****
--- 6352,6363 ----
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 12, "enumvals",
TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 13, "default_val",
+ TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 14, "sourcefile",
+ TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 15, "sourceline",
+ INT4OID, -1, 0);
/*
* Generate attribute metadata needed later to produce tuples from raw
***************
*** 6702,6708 ****
elog(FATAL, "invalid format of exec config params file");
(void) set_config_option(varname, varvalue, record->context,
! varsource, GUC_ACTION_SET, true);
free(varname);
free(varvalue);
}
--- 6829,6835 ----
elog(FATAL, "invalid format of exec config params file");
(void) set_config_option(varname, varvalue, record->context,
! varsource, NULL, 0, GUC_ACTION_SET, true);
free(varname);
free(varvalue);
}
***************
*** 6799,6805 ****
continue;
}
! (void) set_config_option(name, value, context, source, action, true);
free(name);
if (value)
--- 6926,6932 ----
continue;
}
! (void) set_config_option(name, value, context, source, NULL, 0, action, true);
free(name);
if (value)
***************
*** 6826,6832 ****
/* test if the option is valid */
set_config_option(name, value,
superuser() ? PGC_SUSET : PGC_USERSET,
! PGC_S_TEST, GUC_ACTION_SET, false);
/* convert name to canonical spelling, so we can use plain strcmp */
(void) GetConfigOptionByName(name, &varname);
--- 6953,6959 ----
/* test if the option is valid */
set_config_option(name, value,
superuser() ? PGC_SUSET : PGC_USERSET,
! PGC_S_TEST, NULL, 0, GUC_ACTION_SET, false);
/* convert name to canonical spelling, so we can use plain strcmp */
(void) GetConfigOptionByName(name, &varname);
***************
*** 6904,6910 ****
/* test if the option is valid */
set_config_option(name, NULL,
superuser() ? PGC_SUSET : PGC_USERSET,
! PGC_S_TEST, GUC_ACTION_SET, false);
/* convert name to canonical spelling, so we can use plain strcmp */
(void) GetConfigOptionByName(name, &varname);
--- 7031,7037 ----
/* test if the option is valid */
set_config_option(name, NULL,
superuser() ? PGC_SUSET : PGC_USERSET,
! PGC_S_TEST, NULL, 0, GUC_ACTION_SET, false);
/* convert name to canonical spelling, so we can use plain strcmp */
(void) GetConfigOptionByName(name, &varname);
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.512
diff -c -r1.512 pg_proc.h
*** src/include/catalog/pg_proc.h 25 Aug 2008 11:18:43 -0000 1.512
--- src/include/catalog/pg_proc.h 1 Sep 2008 23:27:02 -0000
***************
*** 3157,3163 ****
DESCR("SHOW X as a function");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 f f f f v 3 25 "25 25 16" _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
! DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 f f t t s 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,25}" "{o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals}" show_all_settings _null_ _null_ _null_ ));
DESCR("SHOW ALL as a function");
DATA(insert OID = 1371 ( pg_lock_status PGNSP PGUID 12 1 1000 0 f f t t v 0 2249 "" "{25,26,26,23,21,25,28,26,26,21,25,23,25,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted}" pg_lock_status _null_ _null_ _null_ ));
DESCR("view system lock information");
--- 3157,3163 ----
DESCR("SHOW X as a function");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 f f f f v 3 25 "25 25 16" _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
! DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 f f t t s 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,25,25,25,23}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,default_val,sourcefile,sourceline}" show_all_settings _null_ _null_ _null_ ));
DESCR("SHOW ALL as a function");
DATA(insert OID = 1371 ( pg_lock_status PGNSP PGUID 12 1 1000 0 f f t t v 0 2249 "" "{25,26,26,23,21,25,28,26,26,21,25,23,25,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted}" pg_lock_status _null_ _null_ _null_ ));
DESCR("view system lock information");
Index: src/include/utils/guc.h
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/include/utils/guc.h,v
retrieving revision 1.98
diff -c -r1.98 guc.h
*** src/include/utils/guc.h 23 Jul 2008 17:29:53 -0000 1.98
--- src/include/utils/guc.h 1 Sep 2008 23:27:02 -0000
***************
*** 229,234 ****
--- 229,235 ----
extern bool parse_real(const char *value, double *result);
extern bool set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
+ const char *sourcefile, int sourceline,
GucAction action, bool changeVal);
extern char *GetConfigOptionByName(const char *name, const char **varname);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.41
diff -c -r1.41 guc_tables.h
*** src/include/utils/guc_tables.h 17 Mar 2008 17:45:09 -0000 1.41
--- src/include/utils/guc_tables.h 1 Sep 2008 23:27:02 -0000
***************
*** 126,131 ****
--- 126,135 ----
GucSource reset_source; /* source of the reset_value */
GucSource source; /* source of the current actual value */
GucStack *stack; /* stacked prior values */
+ char *sourcefile; /* file this settings is from (NULL if not file) */
+ int sourceline; /* line in source file */
+ char *reset_sourcefile; /* file that the reset_val is from */
+ int reset_sourceline; /* line in source file for reset_val */
};
/* bit values in flags field */
Index: src/test/regress/expected/rules.out
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/test/regress/expected/rules.out,v
retrieving revision 1.141
diff -c -r1.141 rules.out
*** src/test/regress/expected/rules.out 25 Aug 2008 11:18:43 -0000 1.141
--- src/test/regress/expected/rules.out 1 Sep 2008 23:27:02 -0000
***************
*** 1287,1293 ****
pg_prepared_xacts | SELECT p.transaction, p.gid, p.prepared, u.rolname AS owner, d.datname AS database FROM ((pg_prepared_xact() p(transaction, gid, prepared, ownerid, dbid) LEFT JOIN pg_authid u ON ((p.ownerid = u.oid))) LEFT JOIN pg_database d ON ((p.dbid = d.oid)));
pg_roles | SELECT pg_authid.rolname, pg_authid.rolsuper, pg_authid.rolinherit, pg_authid.rolcreaterole, pg_authid.rolcreatedb, pg_authid.rolcatupdate, pg_authid.rolcanlogin, pg_authid.rolconnlimit, '********'::text AS rolpassword, pg_authid.rolvaliduntil, pg_authid.rolconfig, pg_authid.oid FROM pg_authid;
pg_rules | SELECT n.nspname AS schemaname, c.relname AS tablename, r.rulename, pg_get_ruledef(r.oid) AS definition FROM ((pg_rewrite r JOIN pg_class c ON ((c.oid = r.ev_class))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (r.rulename <> '_RETURN'::name);
! pg_settings | SELECT a.name, a.setting, a.unit, a.category, a.short_desc, a.extra_desc, a.context, a.vartype, a.source, a.min_val, a.max_val, a.enumvals FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals);
pg_shadow | SELECT pg_authid.rolname AS usename, pg_authid.oid AS usesysid, pg_authid.rolcreatedb AS usecreatedb, pg_authid.rolsuper AS usesuper, pg_authid.rolcatupdate AS usecatupd, pg_authid.rolpassword AS passwd, (pg_authid.rolvaliduntil)::abstime AS valuntil, pg_authid.rolconfig AS useconfig FROM pg_authid WHERE pg_authid.rolcanlogin;
pg_stat_activity | SELECT s.datid, d.datname, s.procpid, s.usesysid, u.rolname AS usename, s.current_query, s.waiting, s.xact_start, s.query_start, s.backend_start, s.client_addr, s.client_port FROM pg_database d, pg_stat_get_activity(NULL::integer) s(datid, procpid, usesysid, current_query, waiting, xact_start, query_start, backend_start, client_addr, client_port), pg_authid u WHERE ((s.datid = d.oid) AND (s.usesysid = u.oid));
pg_stat_all_indexes | SELECT c.oid AS relid, i.oid AS indexrelid, n.nspname AS schemaname, c.relname, i.relname AS indexrelname, pg_stat_get_numscans(i.oid) AS idx_scan, pg_stat_get_tuples_returned(i.oid) AS idx_tup_read, pg_stat_get_tuples_fetched(i.oid) AS idx_tup_fetch FROM (((pg_class c JOIN pg_index x ON ((c.oid = x.indrelid))) JOIN pg_class i ON ((i.oid = x.indexrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char"]));
--- 1287,1293 ----
pg_prepared_xacts | SELECT p.transaction, p.gid, p.prepared, u.rolname AS owner, d.datname AS database FROM ((pg_prepared_xact() p(transaction, gid, prepared, ownerid, dbid) LEFT JOIN pg_authid u ON ((p.ownerid = u.oid))) LEFT JOIN pg_database d ON ((p.dbid = d.oid)));
pg_roles | SELECT pg_authid.rolname, pg_authid.rolsuper, pg_authid.rolinherit, pg_authid.rolcreaterole, pg_authid.rolcreatedb, pg_authid.rolcatupdate, pg_authid.rolcanlogin, pg_authid.rolconnlimit, '********'::text AS rolpassword, pg_authid.rolvaliduntil, pg_authid.rolconfig, pg_authid.oid FROM pg_authid;
pg_rules | SELECT n.nspname AS schemaname, c.relname AS tablename, r.rulename, pg_get_ruledef(r.oid) AS definition FROM ((pg_rewrite r JOIN pg_class c ON ((c.oid = r.ev_class))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (r.rulename <> '_RETURN'::name);
! pg_settings | SELECT a.name, a.setting, a.unit, a.category, a.short_desc, a.extra_desc, a.context, a.vartype, a.source, a.min_val, a.max_val, a.enumvals, a.default_val, a.sourcefile, a.sourceline FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals, default_val, sourcefile, sourceline);
pg_shadow | SELECT pg_authid.rolname AS usename, pg_authid.oid AS usesysid, pg_authid.rolcreatedb AS usecreatedb, pg_authid.rolsuper AS usesuper, pg_authid.rolcatupdate AS usecatupd, pg_authid.rolpassword AS passwd, (pg_authid.rolvaliduntil)::abstime AS valuntil, pg_authid.rolconfig AS useconfig FROM pg_authid WHERE pg_authid.rolcanlogin;
pg_stat_activity | SELECT s.datid, d.datname, s.procpid, s.usesysid, u.rolname AS usename, s.current_query, s.waiting, s.xact_start, s.query_start, s.backend_start, s.client_addr, s.client_port FROM pg_database d, pg_stat_get_activity(NULL::integer) s(datid, procpid, usesysid, current_query, waiting, xact_start, query_start, backend_start, client_addr, client_port), pg_authid u WHERE ((s.datid = d.oid) AND (s.usesysid = u.oid));
pg_stat_all_indexes | SELECT c.oid AS relid, i.oid AS indexrelid, n.nspname AS schemaname, c.relname, i.relname AS indexrelname, pg_stat_get_numscans(i.oid) AS idx_scan, pg_stat_get_tuples_returned(i.oid) AS idx_tup_read, pg_stat_get_tuples_fetched(i.oid) AS idx_tup_fetch FROM (((pg_class c JOIN pg_index x ON ((c.oid = x.indrelid))) JOIN pg_class i ON ((i.oid = x.indexrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char"]));
Alvaro Herrera <alvherre@commandprompt.com> writes:
Greg just sent me this patch, augmenting the one I sent to add source
file and line to GUC vars; Greg's patch adds a column with the default
value of each var.
I haven't tested, but doesn't this lose the source-location information
if a setting acquired from the config file is temporarily overridden via
SET (consider SET LOCAL, or a SET in a rolled-back xact)? It'll go to
NULL and not come back.
Since there is no possibility that any module outside GUC should ever
supply a config file location, I don't think that changing the API for
set_config_option is a good idea. Instead have ProcessConfigFile()
call some internal function that's not used by anyone else, and let
set_config_option assume that it's setting a non-file-sourced value.
That'd reduce the footprint of the patch quite a bit.
I dislike using the terminology "default" so cavalierly, because that
is a fairly slippery concept in GUC. Default for whom, with respect
to what? It looks like the patch actually means it to be the boot_val,
but I think a lot of users would think that "default" refers to the
reset value, ie, what their setting will be if they haven't said SET.
The fact that the session default didn't necessarily come from the
config file (see ALTER USER SET, ALTER DATABASE SET) complicates matters
still more. *Please* use some other word than "default" to title that
column. Also, I think that a reasonable case could be made for exposing
both boot_val and reset_val in the view --- if there is a use for one,
there is likely to be a use for the other.
regards, tom lane
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
Greg just sent me this patch, augmenting the one I sent to add source
file and line to GUC vars; Greg's patch adds a column with the default
value of each var.I haven't tested, but doesn't this lose the source-location information
if a setting acquired from the config file is temporarily overridden via
SET (consider SET LOCAL, or a SET in a rolled-back xact)? It'll go to
NULL and not come back.
Hmm, I didn't recheck after Greg's patch, but in mine, it doesn't,
because the location is saved as "reset location" and restored when the
variable is reset. It worked fine in all cases I tested.
Since there is no possibility that any module outside GUC should ever
supply a config file location, I don't think that changing the API for
set_config_option is a good idea. Instead have ProcessConfigFile()
call some internal function that's not used by anyone else, and let
set_config_option assume that it's setting a non-file-sourced value.
That'd reduce the footprint of the patch quite a bit.
Will look into it. FWIW I think most of the callers of
set_config_option are already abusing the API, because they should be
calling SetConfigOption instead. Maybe this needs some cleanup.
Also, I think that a reasonable case could be made for exposing
both boot_val and reset_val in the view --- if there is a use for one,
there is likely to be a use for the other.
How about having two new columns "reset value" and "boot value"?
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
Tom Lane wrote:
I haven't tested, but doesn't this lose the source-location information
if a setting acquired from the config file is temporarily overridden via
SET (consider SET LOCAL, or a SET in a rolled-back xact)? It'll go to
NULL and not come back.
Hmm, I didn't recheck after Greg's patch, but in mine, it doesn't,
because the location is saved as "reset location" and restored when the
variable is reset. It worked fine in all cases I tested.
Hmm. Actually, why is there a need to save and restore at all? There
can certainly never be more than one recorded config-file location per
variable. What about saying that each variable has one and only one
filename/linenumber, but whether these are relevant to the current value
is determined by whether the current value's source is S_FILE?
(This would help to address one of the other things I find annoying
about the patch, which is the amount of storage it eats up for N copies
of what will always be the same filename in practice...)
Will look into it. FWIW I think most of the callers of
set_config_option are already abusing the API, because they should be
calling SetConfigOption instead. Maybe this needs some cleanup.
Yeah, could be. Maybe set_config_option shouldn't be declared in guc.h?
Also, I think that a reasonable case could be made for exposing
both boot_val and reset_val in the view --- if there is a use for one,
there is likely to be a use for the other.
How about having two new columns "reset value" and "boot value"?
Like it better than "default value" ...
regards, tom lane
On Tue, 2 Sep 2008, Tom Lane wrote:
How about having two new columns "reset value" and "boot value"?
Like it better than "default value" ...
It's being a bit pedantic at the expense of the user, but I don't really
care that much here. I exposed the boot_val and described it in the
documentation as:
"Default value if the parameter is not explicitly set"
That's the value that people care about--if they comment out a setting
altogether and restart the server, what will it go back to. New admins
and people playing with the config files in a tuning content aren't often
using sighup in my experience, they just restart the server after changes.
I'm not aware of any specific use case for exposing the reset value other
than for completeness sake. Having both exposed with names that don't
mean anything to new admins is making the user experience more difficult
than it needs to be. That was why I just picked the more important one
and named it "default"; that makes the case for the average user so easy
they don't even need to look at the documentation.
I note the ongoing GUC units debate as a reminder that a technically
correct UI is usually preferred in this project to an easier to use but
slightly ambiguous one, and I'm not going to argue for "default" further
if everyone else is happy with a cryptic naming instead. The important
thing is that the boot_val gets exposed somehow so tool writers can
trivially present it as an option.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Greg Smith <gsmith@gregsmith.com> writes:
On Tue, 2 Sep 2008, Tom Lane wrote:
How about having two new columns "reset value" and "boot value"?
Like it better than "default value" ...
It's being a bit pedantic at the expense of the user, but I don't really
care that much here. I exposed the boot_val and described it in the
documentation as:
"Default value if the parameter is not explicitly set"
If that statement were the truth, the whole truth, and nothing but the
truth, and if it didn't ignore the point about "explicitly set WHERE?",
I'd be fine with it.
That was why I just picked the more important one
and named it "default";
More important to whom? You are adopting a very narrow mindset,
which seems to be that only DBAs look at this view.
regards, tom lane
On Wed, 3 Sep 2008, Tom Lane wrote:
"Default value if the parameter is not explicitly set"
If that statement were the truth, the whole truth, and nothing but the
truth, and if it didn't ignore the point about "explicitly set WHERE?",
I'd be fine with it.
First question--how about if I changed that description to read:
"Default value used at server startup if the parameter is not explicitly
set"?
I could then expose reset-val, named like that and with a description that
explained the context it applies in. And then we've give people a way to
experiment and understand the FAQ of "why didn't the value go back to the
default when I commented it out of the postgresql.conf and HUP'd the
server?".
Section question: with those changes, would it then be reasonable to you
to keep that column named "default" instead of giving it a less common
name?
You are adopting a very narrow mindset, which seems to be that only DBAs
look at this view.
DBAs are the only group I am always getting questions in this area from.
Everybody else seemed happy with the status quo, where the value wasn't
exposed at all and you just looked in guc.c to see what it was.
About once a month, somebody asks me "how can I tell what the default is
for *X*?" I want to be able to answer this question with "look in
pg_settings", which is easy enough to remember, and not have to say
anything else. That's the source of my mindset here, and I'm sure I'm not
alone in fielding that so often.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
On Wed, 3 Sep 2008 16:04:12 -0400 (EDT)
Greg Smith <gsmith@gregsmith.com> wrote:
Section question: with those changes, would it then be reasonable to
you to keep that column named "default" instead of giving it a less
common name?You are adopting a very narrow mindset, which seems to be that only
DBAs look at this view.DBAs are the only group I am always getting questions in this area
from. Everybody else seemed happy with the status quo, where the
value wasn't exposed at all and you just looked in guc.c to see what
it was.
I guess I would ask, "Who else would we be targeting this for?". DBAs
seem to be the only logical choice.
Joshua D. Drake
--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
Joshua Drake <jd@commandprompt.com> writes:
I guess I would ask, "Who else would we be targeting this for?". DBAs
seem to be the only logical choice.
Regular users look at pg_settings too, you know. Maybe *you* only
get questions from DBAs...
regards, tom lane
Greg Smith <gsmith@gregsmith.com> writes:
First question--how about if I changed that description to read:
"Default value used at server startup if the parameter is not explicitly
set"?
"... not otherwise set" would probably be an accurate phrasing.
(I'm thinking of corner cases like stuff absorbed from environment
variables, which aren't really "explicitly" set by any normal usage
of that term.)
I could then expose reset-val, named like that and with a description that
explained the context it applies in. And then we've give people a way to
experiment and understand the FAQ of "why didn't the value go back to the
default when I commented it out of the postgresql.conf and HUP'd the
server?".
You do know that's an ex-FAQ as of 8.3? If we're designing this feature
to respond to that, we are wasting a lot of effort.
About once a month, somebody asks me "how can I tell what the default is
for *X*?"
I wonder how certain you can be of which meaning of "default" they have
in mind. I don't think it means the same thing to everybody that it
means to you.
regards, tom lane
Before I respond to Tom's comments, let me step back a second and add the
intro the deadline didn't leave me time for. There are two specific
things the bit I added to this GUC patch is aimed at:
1) Somebody has a postgresql.conf from a random source (saw it on the
Internet and pasted dubious stuff in/previous person working on the
server/etc.) and wants to know the default value they'd get if they just
commented a specific line or lines out.
2) A GUC tuning tool author wants to provide a UI for modifying a GUC
parameter that shows the default as input to the person deciding what to
set a parameter to. The interface I've always wanted to make available
would be...wait a minute, I can provide a working example now. Picture
this:
name | Recommended | Current | Min | Default | Max
-------------+-------------+---------+-------+---------+---------
wal_buffers | 1024kB | 64kB | 32 kB | 64 kB | 2048 MB
With your cursor lighting up either the "Recommended" or "Current" field,
depending on whether you're a default approve or deny kind of tool
designer. Pretty simple interface to decide what to do, right? I find
that much less useful without the default value being included, but right
now someone who is writing a tuning tool has to maintain their own
database with that information if they want to do that. I will actually
do that for earlier versions the minute I know what the 8.4 solution that
makes the problem go away looks like.
The above is the output from:
select name,
'1024kB' as "Recommended",
current_setting(name) as "Current",
case when unit='8kB' then pg_size_pretty(min_val::int8*8192) else
min_val end as "Min",
case when unit='8kB' then pg_size_pretty(default_val::int8*8192) else
default_val end as "Default",
case when unit='8kB' then pg_size_pretty(max_val::int8) else max_val end
as "Max"
from pg_settings where name='wal_buffers';
on my system with the patch installed.
That's what I wanted but was unable to get until now. Combine that with
being able to figure out what source file and line the setting was
actually taken from, and the top 3 obstacles to writing a simple and easy
to use read/modify/write tuning tool are all cleared.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
On Wed, 3 Sep 2008, Tom Lane wrote:
Greg Smith <gsmith@gregsmith.com> writes:
First question--how about if I changed that description to read:
"Default value used at server startup if the parameter is not explicitly
set"?"... not otherwise set" would probably be an accurate phrasing.
(I'm thinking of corner cases like stuff absorbed from environment
variables, which aren't really "explicitly" set by any normal usage
of that term.)
My opinion is that setting something in an environment variable certainly
is explicitly setting it, but it doesn't matter; "if the parameter is not
otherwise set" works just as well as far as I'm concerned.
I could then expose reset-val, named like that and with a description that
explained the context it applies in. And then we've give people a way to
experiment and understand the FAQ of "why didn't the value go back to the
default...You do know that's an ex-FAQ as of 8.3? If we're designing this feature
to respond to that, we are wasting a lot of effort.
Sure, but there are a lot of pre-8.3 installs out there. I don't really
care about the reset-val at all, so I'm not going to justify whether or
not it should be included.
I wonder how certain you can be of which meaning of "default" they have
in mind. I don't think it means the same thing to everybody that it
means to you.
When most people say "the default" talking about a value in a
configuration file, they mean the value the software will assume if that
setting isn't there at all. In the postgresql.conf context, that means
what they'll get if they start the server with that line missing or
commented out (and no environment variables, etc.) which is why I mapped
that to the boot_val. While I'm aware there are other uses of "default"
that apply in this context, I think they are extremely rare compared to
the common usage. The subtle distictions that require both a boot_val and
a reset_val internally are only important to people who are also capable
of understanding that "default" is a mass-consumption oriented label
that's a touch fuzzy IMHO.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Greg Smith <gsmith@gregsmith.com> wrote:
name | Recommended | Current | Min | Default | Max
-------------+-------------+---------+-------+---------+---------
wal_buffers | 1024kB | 64kB | 32 kB | 64 kB | 2048 MB
Personally, I would take the "Min", "Default", and "Max" to mean what
Greg intends; it's the "Current" one that gives me pause. The current
value of this connection? The value that a new connection will
currently get? The value which new connections will get after a
reload with the current conf file? The value which new connections
will get after a restart with the current conf file? I can understand
how someone would take one of these four values to be what is meant by
"Default", though.
-Kevin
On Thu, 4 Sep 2008, Kevin Grittner wrote:
Personally, I would take the "Min", "Default", and "Max" to mean what
Greg intends; it's the "Current" one that gives me pause.
That's the output of current_setting(name) which shows what it is right
now; no more, no less. See
http://www.postgresql.org/docs/current/interactive/sql-show.html and
http://www.postgresql.org/docs/current/interactive/functions-admin.html
That shows the parameter as the admin/usr set it, whereas the "setting"
column in pg_settings displays in terms of the internal units. Example
session showing how that all fits together:
pg=# select current_setting('work_mem');
current_setting
-----------------
1GB
(1 row)
pg=# set work_mem='256MB';
SET
pg=# select current_setting('work_mem');
current_setting
-----------------
256MB
(1 row)
pg=# select setting,unit from pg_settings where name='work_mem';
setting | unit
---------+------
262144 | kB
(1 row)
Since the word "current" isn't actually in the patch anywhere, and only
appears in that little sample usage snippet I provided, whether or not
it's a good name for that doesn't impact the patch itself.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
Tom Lane wrote:
Hmm, I didn't recheck after Greg's patch, but in mine, it doesn't,
because the location is saved as "reset location" and restored when the
variable is reset. It worked fine in all cases I tested.Hmm. Actually, why is there a need to save and restore at all? There
can certainly never be more than one recorded config-file location per
variable. What about saying that each variable has one and only one
filename/linenumber, but whether these are relevant to the current value
is determined by whether the current value's source is S_FILE?
Hmm, this does make the patch a lot simpler. Attached. (Magnus was
visionary enough to put the correct test in the pg_settings definition.)
I also dropped the change to set_config_option, and added a new routine
to set the source file/line, as you suggested elsewhere. The only
problem is that we now have two bsearch calls for each option set in a
config file ... I don't want to change set_config_option just to be
able to return the struct config_generic for this routine's sake ...
Better ideas? Is this OK as is?
I noticed some weird things when the config files contain errors, but I
think it's outside this patch's scope.
(I dropped the "default" stuff for now, as it doesn't seem that a
consensus has been reached on that topic.)
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachments:
guc-sourcefile-4.patchtext/x-diff; charset=us-asciiDownload
Index: doc/src/sgml/catalogs.sgml
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/doc/src/sgml/catalogs.sgml,v
retrieving revision 2.172
diff -c -p -r2.172 catalogs.sgml
*** doc/src/sgml/catalogs.sgml 30 Jul 2008 17:05:04 -0000 2.172
--- doc/src/sgml/catalogs.sgml 9 Sep 2008 02:42:14 -0000
***************
*** 6414,6419 ****
--- 6414,6433 ----
<entry>Allowed values in enum parameters (NULL for non-enum
values)</entry>
</row>
+ <row>
+ <entry><structfield>sourcefile</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>Input file the current value was set from (NULL for values set in
+ sources other than configuration files). Helpful when using
+ configuration include directives.</entry>
+ </row>
+ <row>
+ <entry><structfield>sourceline</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>Line number within the sourcefile the current value was set
+ from (NULL for values set in sources other than configuration files)
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.56
diff -c -p -r1.56 guc-file.l
*** src/backend/utils/misc/guc-file.l 22 Aug 2008 00:20:40 -0000 1.56
--- src/backend/utils/misc/guc-file.l 9 Sep 2008 02:09:28 -0000
*************** struct name_value_pair
*** 39,44 ****
--- 39,46 ----
{
char *name;
char *value;
+ char *filename;
+ int sourceline;
struct name_value_pair *next;
};
*************** ProcessConfigFile(GucContext context)
*** 307,314 ****
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
! set_config_option(item->name, item->value, context,
! PGC_S_FILE, GUC_ACTION_SET, true);
}
/* Remember when we last successfully loaded the config file. */
--- 309,320 ----
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
! if (set_config_option(item->name, item->value, context,
! PGC_S_FILE, GUC_ACTION_SET, true))
! {
! set_config_sourcefile(item->name, item->filename,
! item->sourceline);
! }
}
/* Remember when we last successfully loaded the config file. */
*************** ParseConfigFile(const char *config_file,
*** 483,488 ****
--- 489,496 ----
pfree(item->value);
item->name = opt_name;
item->value = opt_value;
+ item->filename = pstrdup(config_file);
+ item->sourceline = ConfigFileLineno-1;
}
else
{
*************** ParseConfigFile(const char *config_file,
*** 490,495 ****
--- 498,505 ----
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
+ item->filename = pstrdup(config_file);
+ item->sourceline = ConfigFileLineno-1;
item->next = *head_p;
*head_p = item;
if (*tail_p == NULL)
*************** ParseConfigFile(const char *config_file,
*** 502,507 ****
--- 512,519 ----
item = palloc(sizeof *item);
item->name = opt_name;
item->value = opt_value;
+ item->filename = pstrdup(config_file);
+ item->sourceline = ConfigFileLineno-1;
item->next = NULL;
if (*head_p == NULL)
*head_p = item;
*************** free_name_value_list(struct name_value_p
*** 553,558 ****
--- 565,571 ----
pfree(item->name);
pfree(item->value);
+ pfree(item->filename);
pfree(item);
item = next;
}
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.470
diff -c -p -r1.470 guc.c
*** src/backend/utils/misc/guc.c 25 Aug 2008 15:11:00 -0000 1.470
--- src/backend/utils/misc/guc.c 9 Sep 2008 02:24:27 -0000
*************** extern bool optimize_bounded_sort;
*** 129,134 ****
--- 129,136 ----
extern char *SSLCipherSuites;
#endif
+ static void set_config_sourcefile(const char *name, char *sourcefile,
+ int sourceline);
static const char *assign_log_destination(const char *value,
bool doit, GucSource source);
*************** InitializeGUCOptions(void)
*** 3200,3205 ****
--- 3202,3209 ----
gconf->reset_source = PGC_S_DEFAULT;
gconf->source = PGC_S_DEFAULT;
gconf->stack = NULL;
+ gconf->sourcefile = NULL;
+ gconf->sourceline = 0;
switch (gconf->vartype)
{
*************** ResetAllOptions(void)
*** 3541,3547 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_INT:
--- 3545,3550 ----
*************** ResetAllOptions(void)
*** 3553,3559 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_REAL:
--- 3556,3561 ----
*************** ResetAllOptions(void)
*** 3565,3571 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_STRING:
--- 3567,3572 ----
*************** ResetAllOptions(void)
*** 3594,3600 ****
}
set_string_field(conf, conf->variable, str);
- conf->gen.source = conf->gen.reset_source;
break;
}
case PGC_ENUM:
--- 3595,3600 ----
*************** ResetAllOptions(void)
*** 3606,3616 ****
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
- conf->gen.source = conf->gen.reset_source;
break;
}
}
if (gconf->flags & GUC_REPORT)
ReportGUCOption(gconf);
}
--- 3606,3617 ----
PGC_S_SESSION))
elog(ERROR, "failed to reset %s", conf->gen.name);
*conf->variable = conf->reset_val;
break;
}
}
+ gconf->source = gconf->reset_source;
+
if (gconf->flags & GUC_REPORT)
ReportGUCOption(gconf);
}
*************** set_config_option(const char *name, cons
*** 5108,5116 ****
--- 5109,5146 ----
/*
+ * Set the fields for source file and line number the setting came from.
+ */
+ static void
+ set_config_sourcefile(const char *name, char *sourcefile, int sourceline)
+ {
+ struct config_generic *record;
+ int elevel;
+
+ /*
+ * To avoid cluttering the log, only the postmaster bleats loudly
+ * about problems with the config file.
+ */
+ elevel = IsUnderPostmaster ? DEBUG3 : LOG;
+
+ record = find_option(name, true, elevel);
+ /* should not happen */
+ if (record == NULL)
+ elog(ERROR, "unrecognized configuration parameter \"%s\"", name);
+
+ if (record->sourcefile)
+ free(record->sourcefile);
+ record->sourcefile = guc_strdup(elevel, sourcefile);
+ record->sourceline = sourceline;
+ }
+
+ /*
* Set a config option to the given value. See also set_config_option,
* this is just the wrapper to be called from outside GUC. NB: this
* is used only for non-transactional operations.
+ *
+ * Note: there is no support here for setting source file/line, as it
+ * is currently not needed.
*/
void
SetConfigOption(const char *name, const char *value,
*************** GetConfigOptionByNum(int varnum, const c
*** 6144,6149 ****
--- 6174,6192 ----
}
break;
}
+
+ /* If the setting came from a config file, set the source location */
+ if (conf->source == PGC_S_FILE)
+ {
+ values[12] = conf->sourcefile;
+ snprintf(buffer, sizeof(buffer), "%d", conf->sourceline);
+ values[13] = pstrdup(buffer);
+ }
+ else
+ {
+ values[12] = NULL;
+ values[13] = NULL;
+ }
}
/*
*************** show_config_by_name(PG_FUNCTION_ARGS)
*** 6179,6185 ****
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
! #define NUM_PG_SETTINGS_ATTS 12
Datum
show_all_settings(PG_FUNCTION_ARGS)
--- 6222,6228 ----
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
! #define NUM_PG_SETTINGS_ATTS 14
Datum
show_all_settings(PG_FUNCTION_ARGS)
*************** show_all_settings(PG_FUNCTION_ARGS)
*** 6231,6236 ****
--- 6274,6283 ----
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 12, "enumvals",
TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 13, "sourcefile",
+ TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 14, "sourceline",
+ INT4OID, -1, 0);
/*
* Generate attribute metadata needed later to produce tuples from raw
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.513
diff -c -p -r1.513 pg_proc.h
*** src/include/catalog/pg_proc.h 6 Sep 2008 00:01:24 -0000 1.513
--- src/include/catalog/pg_proc.h 9 Sep 2008 00:11:29 -0000
*************** DATA(insert OID = 2077 ( current_settin
*** 3159,3165 ****
DESCR("SHOW X as a function");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 f f f f v 3 25 "25 25 16" _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
! DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 f f t t s 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,25}" "{o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals}" show_all_settings _null_ _null_ _null_ ));
DESCR("SHOW ALL as a function");
DATA(insert OID = 1371 ( pg_lock_status PGNSP PGUID 12 1 1000 0 f f t t v 0 2249 "" "{25,26,26,23,21,25,28,26,26,21,25,23,25,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted}" pg_lock_status _null_ _null_ _null_ ));
DESCR("view system lock information");
--- 3159,3165 ----
DESCR("SHOW X as a function");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 f f f f v 3 25 "25 25 16" _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
! DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 f f t t s 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,25,25,23}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,sourcefile,sourceline}" show_all_settings _null_ _null_ _null_ ));
DESCR("SHOW ALL as a function");
DATA(insert OID = 1371 ( pg_lock_status PGNSP PGUID 12 1 1000 0 f f t t v 0 2249 "" "{25,26,26,23,21,25,28,26,26,21,25,23,25,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted}" pg_lock_status _null_ _null_ _null_ ));
DESCR("view system lock information");
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.41
diff -c -p -r1.41 guc_tables.h
*** src/include/utils/guc_tables.h 17 Mar 2008 17:45:09 -0000 1.41
--- src/include/utils/guc_tables.h 9 Sep 2008 00:23:27 -0000
*************** struct config_generic
*** 126,131 ****
--- 126,133 ----
GucSource reset_source; /* source of the reset_value */
GucSource source; /* source of the current actual value */
GucStack *stack; /* stacked prior values */
+ char *sourcefile; /* file this settings is from (NULL if not file) */
+ int sourceline; /* line in source file */
};
/* bit values in flags field */
Index: src/test/regress/expected/rules.out
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/test/regress/expected/rules.out,v
retrieving revision 1.141
diff -c -p -r1.141 rules.out
*** src/test/regress/expected/rules.out 25 Aug 2008 11:18:43 -0000 1.141
--- src/test/regress/expected/rules.out 9 Sep 2008 00:11:29 -0000
*************** SELECT viewname, definition FROM pg_view
*** 1287,1293 ****
pg_prepared_xacts | SELECT p.transaction, p.gid, p.prepared, u.rolname AS owner, d.datname AS database FROM ((pg_prepared_xact() p(transaction, gid, prepared, ownerid, dbid) LEFT JOIN pg_authid u ON ((p.ownerid = u.oid))) LEFT JOIN pg_database d ON ((p.dbid = d.oid)));
pg_roles | SELECT pg_authid.rolname, pg_authid.rolsuper, pg_authid.rolinherit, pg_authid.rolcreaterole, pg_authid.rolcreatedb, pg_authid.rolcatupdate, pg_authid.rolcanlogin, pg_authid.rolconnlimit, '********'::text AS rolpassword, pg_authid.rolvaliduntil, pg_authid.rolconfig, pg_authid.oid FROM pg_authid;
pg_rules | SELECT n.nspname AS schemaname, c.relname AS tablename, r.rulename, pg_get_ruledef(r.oid) AS definition FROM ((pg_rewrite r JOIN pg_class c ON ((c.oid = r.ev_class))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (r.rulename <> '_RETURN'::name);
! pg_settings | SELECT a.name, a.setting, a.unit, a.category, a.short_desc, a.extra_desc, a.context, a.vartype, a.source, a.min_val, a.max_val, a.enumvals FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals);
pg_shadow | SELECT pg_authid.rolname AS usename, pg_authid.oid AS usesysid, pg_authid.rolcreatedb AS usecreatedb, pg_authid.rolsuper AS usesuper, pg_authid.rolcatupdate AS usecatupd, pg_authid.rolpassword AS passwd, (pg_authid.rolvaliduntil)::abstime AS valuntil, pg_authid.rolconfig AS useconfig FROM pg_authid WHERE pg_authid.rolcanlogin;
pg_stat_activity | SELECT s.datid, d.datname, s.procpid, s.usesysid, u.rolname AS usename, s.current_query, s.waiting, s.xact_start, s.query_start, s.backend_start, s.client_addr, s.client_port FROM pg_database d, pg_stat_get_activity(NULL::integer) s(datid, procpid, usesysid, current_query, waiting, xact_start, query_start, backend_start, client_addr, client_port), pg_authid u WHERE ((s.datid = d.oid) AND (s.usesysid = u.oid));
pg_stat_all_indexes | SELECT c.oid AS relid, i.oid AS indexrelid, n.nspname AS schemaname, c.relname, i.relname AS indexrelname, pg_stat_get_numscans(i.oid) AS idx_scan, pg_stat_get_tuples_returned(i.oid) AS idx_tup_read, pg_stat_get_tuples_fetched(i.oid) AS idx_tup_fetch FROM (((pg_class c JOIN pg_index x ON ((c.oid = x.indrelid))) JOIN pg_class i ON ((i.oid = x.indexrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char"]));
--- 1287,1293 ----
pg_prepared_xacts | SELECT p.transaction, p.gid, p.prepared, u.rolname AS owner, d.datname AS database FROM ((pg_prepared_xact() p(transaction, gid, prepared, ownerid, dbid) LEFT JOIN pg_authid u ON ((p.ownerid = u.oid))) LEFT JOIN pg_database d ON ((p.dbid = d.oid)));
pg_roles | SELECT pg_authid.rolname, pg_authid.rolsuper, pg_authid.rolinherit, pg_authid.rolcreaterole, pg_authid.rolcreatedb, pg_authid.rolcatupdate, pg_authid.rolcanlogin, pg_authid.rolconnlimit, '********'::text AS rolpassword, pg_authid.rolvaliduntil, pg_authid.rolconfig, pg_authid.oid FROM pg_authid;
pg_rules | SELECT n.nspname AS schemaname, c.relname AS tablename, r.rulename, pg_get_ruledef(r.oid) AS definition FROM ((pg_rewrite r JOIN pg_class c ON ((c.oid = r.ev_class))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (r.rulename <> '_RETURN'::name);
! pg_settings | SELECT a.name, a.setting, a.unit, a.category, a.short_desc, a.extra_desc, a.context, a.vartype, a.source, a.min_val, a.max_val, a.enumvals, a.sourcefile, a.sourceline FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals, sourcefile, sourceline);
pg_shadow | SELECT pg_authid.rolname AS usename, pg_authid.oid AS usesysid, pg_authid.rolcreatedb AS usecreatedb, pg_authid.rolsuper AS usesuper, pg_authid.rolcatupdate AS usecatupd, pg_authid.rolpassword AS passwd, (pg_authid.rolvaliduntil)::abstime AS valuntil, pg_authid.rolconfig AS useconfig FROM pg_authid WHERE pg_authid.rolcanlogin;
pg_stat_activity | SELECT s.datid, d.datname, s.procpid, s.usesysid, u.rolname AS usename, s.current_query, s.waiting, s.xact_start, s.query_start, s.backend_start, s.client_addr, s.client_port FROM pg_database d, pg_stat_get_activity(NULL::integer) s(datid, procpid, usesysid, current_query, waiting, xact_start, query_start, backend_start, client_addr, client_port), pg_authid u WHERE ((s.datid = d.oid) AND (s.usesysid = u.oid));
pg_stat_all_indexes | SELECT c.oid AS relid, i.oid AS indexrelid, n.nspname AS schemaname, c.relname, i.relname AS indexrelname, pg_stat_get_numscans(i.oid) AS idx_scan, pg_stat_get_tuples_returned(i.oid) AS idx_tup_read, pg_stat_get_tuples_fetched(i.oid) AS idx_tup_fetch FROM (((pg_class c JOIN pg_index x ON ((c.oid = x.indrelid))) JOIN pg_class i ON ((i.oid = x.indexrelid))) LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace))) WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char"]));
On Mon, 8 Sep 2008, Alvaro Herrera wrote:
(I dropped the "default" stuff for now, as it doesn't seem that a
consensus has been reached on that topic.)
I have multiple GUC-related projects that are all stalled waiting for that
capability to be added. The only thing there wasn't clear consensus on
was exactly what the name for it should be, and there I really don't care.
I made the argument for why I named it the way I did, but if it gets
committed with a less friendly name (like boot_val) I won't complain.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Alvaro Herrera wrote:
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
Tom Lane wrote:
Hmm, I didn't recheck after Greg's patch, but in mine, it doesn't,
because the location is saved as "reset location" and restored when the
variable is reset. It worked fine in all cases I tested.Hmm. Actually, why is there a need to save and restore at all? There
can certainly never be more than one recorded config-file location per
variable. What about saying that each variable has one and only one
filename/linenumber, but whether these are relevant to the current value
is determined by whether the current value's source is S_FILE?Hmm, this does make the patch a lot simpler. Attached. (Magnus was
visionary enough to put the correct test in the pg_settings definition.)
:-)
Yeah, it does look at lot simpler. And it certainly takes away the
pieces of code of mine that I was entirely unable to make working :-)
I also dropped the change to set_config_option, and added a new routine
to set the source file/line, as you suggested elsewhere. The only
problem is that we now have two bsearch calls for each option set in a
config file ... I don't want to change set_config_option just to be
able to return the struct config_generic for this routine's sake ...
Better ideas? Is this OK as is?
Well, it's not like it's a performance critical path, is it? I think we
should be ok.
I noticed some weird things when the config files contain errors, but I
think it's outside this patch's scope.(I dropped the "default" stuff for now, as it doesn't seem that a
consensus has been reached on that topic.)
This is one of the reasons I suggested keeping that one as a separate
patch in the first place. The other main reason being that once it gets
applied, you really want it to be two different revisions, to clearly
keep them apart ;-) I still think we should eventually get both in
there, but let's treat them as separate entities.
//Magnus
On Tue, 9 Sep 2008, Magnus Hagander wrote:
(I dropped the "default" stuff for now, as it doesn't seem that a
consensus has been reached on that topic.)This is one of the reasons I suggested keeping that one as a separate
patch in the first place. The other main reason being that once it gets
applied, you really want it to be two different revisions, to clearly
keep them apart
This means some committer is going to have to make a second pass over the
same section of code and do testing there more than once, that's a waste
of time I was trying to avoid. Also, any standalone patch I submit right
now won't apply cleanly if the source file/line patch is committed.
If nobody cares about doing that work twice, I'll re-submit a separate
patch once this one is resolved one way or another. I hope you snagged
the documentation update I added to your patch though.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Greg Smith wrote:
On Tue, 9 Sep 2008, Magnus Hagander wrote:
(I dropped the "default" stuff for now, as it doesn't seem that a
consensus has been reached on that topic.)This is one of the reasons I suggested keeping that one as a separate
patch in the first place. The other main reason being that once it gets
applied, you really want it to be two different revisions, to clearly
keep them apartThis means some committer is going to have to make a second pass over the
same section of code and do testing there more than once, that's a waste
of time I was trying to avoid.
Actually, this is done all the time.
Also, any standalone patch I submit right now won't apply cleanly if
the source file/line patch is committed.
You can always start from the patched version and use interdiff to
obtain a "patch difference" ...
If nobody cares about doing that work twice, I'll re-submit a separate
patch once this one is resolved one way or another. I hope you snagged
the documentation update I added to your patch though.
Yeah, I did.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support