Add default_val to pg_settings

Started by Greg Smithover 17 years ago13 messages
#1Greg Smith
gsmith@gregsmith.com
1 attachment(s)

Attached is an updated and separate version of my patch exposing the
internal GUC boot_val as default_val, which failed to attach itself to the
already committed changes to the pg_settings view.

Brief reminder of what it does:

postgres=# select name,setting,default_val from pg_settings where name='shared_buffers';
name | setting | default_val
----------------+---------+-------------
shared_buffers | 4096 | 1024

General justification for this change with a longer example is at
http://archives.postgresql.org/pgsql-hackers/2008-09/msg00233.php

Based on feedback the first time around, I updated the documentation for
this column to read "Default value assumed at server startup if the
parameter is not otherwise set".

Would only take a quick search/replace of the patch to change
"default_val" to something else if there are still any objections there.
"boot_val" is another candidate name, I feel that would make the purpose
of the column less obvious to users of pg_settings even if it is more
correct. I'm really more concerned about the feature than exactly what
it's named though. I didn't bother to expose the reset_val since I can't
think of any obvious use cases for wanting to know it.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD

Attachments:

guc-with-defaults-2.patchtext/plain; CHARSET=US-ASCII; NAME=guc-with-defaults-2.patchDownload
Index: doc/src/sgml/catalogs.sgml
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/doc/src/sgml/catalogs.sgml,v
retrieving revision 2.174
diff -c -r2.174 catalogs.sgml
*** doc/src/sgml/catalogs.sgml	15 Sep 2008 18:43:41 -0000	2.174
--- doc/src/sgml/catalogs.sgml	16 Sep 2008 03:18:42 -0000
***************
*** 6422,6427 ****
--- 6422,6433 ----
        values)</entry>
       </row>
       <row>
+       <entry><structfield>default_val</structfield></entry>
+       <entry><type>text</type></entry>
+       <entry>Default value assumed at server startup if the parameter is not 
+       otherwise set</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
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.472
diff -c -r1.472 guc.c
*** src/backend/utils/misc/guc.c	10 Sep 2008 19:16:22 -0000	1.472
--- src/backend/utils/misc/guc.c	16 Sep 2008 03:44:09 -0000
***************
*** 6087,6092 ****
--- 6087,6094 ----
  	{
  		case PGC_BOOL:
  			{
+ 				struct config_bool *lconf = (struct config_bool *) conf;
+ 
  				/* min_val */
  				values[9] = NULL;
  
***************
*** 6095,6100 ****
--- 6097,6106 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+ 				/* default_val */
+ 				snprintf(buffer, sizeof(buffer), "%s", lconf->boot_val ? "on" : "off");
+ 				values[12] = pstrdup(buffer);
  			}
  			break;
  
***************
*** 6112,6117 ****
--- 6118,6127 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+  				/* default_val */
+  				snprintf(buffer, sizeof(buffer), "%d", lconf->boot_val);
+  				values[12] = pstrdup(buffer);
  			}
  			break;
  
***************
*** 6129,6139 ****
--- 6139,6155 ----
  
  				/* 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;
  
***************
*** 6142,6152 ****
--- 6158,6180 ----
  
  				/* 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;
  
***************
*** 6155,6160 ****
--- 6183,6193 ----
  
  				/* 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;
  
***************
*** 6172,6177 ****
--- 6205,6213 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+  				/* default_val */
+  				values[12] = NULL;
  			}
  			break;
  	}
***************
*** 6179,6192 ****
  	/* 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;
  	}
  }
  
--- 6215,6228 ----
  	/* 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;
  	}
  }
  
***************
*** 6223,6229 ****
   * 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)
--- 6259,6265 ----
   * 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)
***************
*** 6275,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);
  
  		/*
--- 6311,6321 ----
  						   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);
  
  		/*
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.514
diff -c -r1.514 pg_proc.h
*** src/include/catalog/pg_proc.h	10 Sep 2008 18:09:20 -0000	1.514
--- src/include/catalog/pg_proc.h	16 Sep 2008 03:31:53 -0000
***************
*** 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");
--- 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,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/test/regress/expected/rules.out
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/test/regress/expected/rules.out,v
retrieving revision 1.142
diff -c -r1.142 rules.out
*** src/test/regress/expected/rules.out	10 Sep 2008 18:09:20 -0000	1.142
--- src/test/regress/expected/rules.out	16 Sep 2008 03:33:11 -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, 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"]));
--- 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"]));
#2Simon Riggs
simon@2ndQuadrant.com
In reply to: Greg Smith (#1)
Re: Add default_val to pg_settings

On Tue, 2008-09-16 at 00:10 -0400, Greg Smith wrote:

Attached is an updated and separate version of my patch exposing the
internal GUC boot_val as default_val, which failed to attach itself to the
already committed changes to the pg_settings view.

Brief reminder of what it does:

postgres=# select name,setting,default_val from pg_settings where name='shared_buffers';
name | setting | default_val
----------------+---------+-------------
shared_buffers | 4096 | 1024

General justification for this change with a longer example is at
http://archives.postgresql.org/pgsql-hackers/2008-09/msg00233.php

Based on feedback the first time around, I updated the documentation for
this column to read "Default value assumed at server startup if the
parameter is not otherwise set".

Would only take a quick search/replace of the patch to change
"default_val" to something else if there are still any objections there.
"boot_val" is another candidate name, I feel that would make the purpose
of the column less obvious to users of pg_settings even if it is more
correct. I'm really more concerned about the feature than exactly what
it's named though. I didn't bother to expose the reset_val since I can't
think of any obvious use cases for wanting to know it.

We have an RESET command which "returns parameter to its default
setting". But what this means is "return it to the value set in current
the postgresql.conf, if overriden therein from its default value". So it
would be useful to have a column that meant "if I run the RESET command
it would return me to this value".

The boot value is only interesting when the "source" column of
pg_settings is "default". In all other cases it is a misleading value,
AFAICS. It would be accurate in sessions that have not run SET, or have
just issued RESET ALL, but we have no way of knowing whether that is the
case or not.

I would suggest we either alter pg_settings so that we display value
*only* when source=default (set NULL otherwise) or we do some extra work
to derive what the setting would be if we ran RESET. The latter would be
preferred approach.

So column name boot_val seems most correct currently, but not very
useful. If we can make the changes above, then I'd stick with
default_val.

I like the concept very much though so please don't take my words as
opposition. *This* patch should not be applied, but a similar one could
and should be.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#2)
Re: Add default_val to pg_settings

Simon Riggs <simon@2ndQuadrant.com> writes:

We have an RESET command which "returns parameter to its default
setting". But what this means is "return it to the value set in current
the postgresql.conf, if overriden therein from its default value". So it
would be useful to have a column that meant "if I run the RESET command
it would return me to this value".

The boot value is only interesting when the "source" column of
pg_settings is "default". In all other cases it is a misleading value,
AFAICS. It would be accurate in sessions that have not run SET, or have
just issued RESET ALL, but we have no way of knowing whether that is the
case or not.

Right, this is why I was complaining that the view should expose the
reset_val. Greg's opinion that only boot_val is needed seems to be
focused entirely on DBAs or tools for manipulating postgresql.conf ---
the only reason you'd want to know boot_val is to know "what will happen
if I remove this setting from postgresql.conf?". For ordinary users
boot_val is useless information, but reset_val could be interesting.

I would suggest we either alter pg_settings so that we display value
*only* when source=default (set NULL otherwise) or we do some extra work
to derive what the setting would be if we ran RESET. The latter would be
preferred approach.

Trying to make one column serve both masters sounds hopelessly confusing
to me; it would essentially make it useless for *both* sets of users,
because neither would know whether the value they're seeing is the one
they need.

regards, tom lane

#4Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#3)
Re: Add default_val to pg_settings

Tom Lane wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

We have an RESET command which "returns parameter to its default
setting". But what this means is "return it to the value set in current
the postgresql.conf, if overriden therein from its default value". So it
would be useful to have a column that meant "if I run the RESET command
it would return me to this value".

The boot value is only interesting when the "source" column of
pg_settings is "default". In all other cases it is a misleading value,
AFAICS. It would be accurate in sessions that have not run SET, or have
just issued RESET ALL, but we have no way of knowing whether that is the
case or not.

Right, this is why I was complaining that the view should expose the
reset_val. Greg's opinion that only boot_val is needed seems to be
focused entirely on DBAs or tools for manipulating postgresql.conf ---
the only reason you'd want to know boot_val is to know "what will happen
if I remove this setting from postgresql.conf?". For ordinary users
boot_val is useless information, but reset_val could be interesting.

If both are interesting to different audiences, perhaps we should be
exposing both as separate columns?

I would suggest we either alter pg_settings so that we display value
*only* when source=default (set NULL otherwise) or we do some extra work
to derive what the setting would be if we ran RESET. The latter would be
preferred approach.

Trying to make one column serve both masters sounds hopelessly confusing
to me; it would essentially make it useless for *both* sets of users,
because neither would know whether the value they're seeing is the one
they need.

It sounds like you are making the case for what I write above? Having
one column named reset_val and one named boot_val should work, no?

//Magnus

#5Simon Riggs
simon@2ndQuadrant.com
In reply to: Magnus Hagander (#4)
Re: Add default_val to pg_settings

On Thu, 2008-09-25 at 14:42 +0200, Magnus Hagander wrote:

If both are interesting to different audiences, perhaps we should be
exposing both as separate columns?

That seems best. It will make things much clearer.

Having
one column named reset_val and one named boot_val should work, no?

Yes, those names seem very appropriate to me.

Finding the reset_val isn't that tough, IIRC the way the guc assignment
works with its "doit" flag.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

#6Magnus Hagander
magnus@hagander.net
In reply to: Simon Riggs (#5)
Re: Add default_val to pg_settings

Simon Riggs wrote:

On Thu, 2008-09-25 at 14:42 +0200, Magnus Hagander wrote:

Having
one column named reset_val and one named boot_val should work, no?

Yes, those names seem very appropriate to me.

Finding the reset_val isn't that tough, IIRC the way the guc assignment
works with its "doit" flag.

I haven't looked at the code.. but there is actually a field in the
struct called reset_val. It may not be usable, or you may have just
missed it?

//Magnus

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#4)
Re: Add default_val to pg_settings

Magnus Hagander <magnus@hagander.net> writes:

It sounds like you are making the case for what I write above? Having
one column named reset_val and one named boot_val should work, no?

Well, that's what I've been saying right along, but the previous
discussion was all about what to call the columns ...

regards, tom lane

#8Simon Riggs
simon@2ndQuadrant.com
In reply to: Magnus Hagander (#6)
Re: Add default_val to pg_settings

On Thu, 2008-09-25 at 14:52 +0200, Magnus Hagander wrote:

Simon Riggs wrote:

On Thu, 2008-09-25 at 14:42 +0200, Magnus Hagander wrote:

Having
one column named reset_val and one named boot_val should work, no?

Yes, those names seem very appropriate to me.

Finding the reset_val isn't that tough, IIRC the way the guc assignment
works with its "doit" flag.

I haven't looked at the code.. but there is actually a field in the
struct called reset_val. It may not be usable, or you may have just
missed it?

Yah, missed it. Cool, even easier ;-)

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

#9Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#7)
Re: Add default_val to pg_settings

On Thu, 2008-09-25 at 09:15 -0400, Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

It sounds like you are making the case for what I write above? Having
one column named reset_val and one named boot_val should work, no?

Well, that's what I've been saying right along, but the previous
discussion was all about what to call the columns ...

Sorry Tom, I meant I was agreeing with both of you, not just Magnus.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

#10Greg Smith
gsmith@gregsmith.com
In reply to: Simon Riggs (#2)
Re: Add default_val to pg_settings

On Thu, 25 Sep 2008, Simon Riggs wrote:

I would suggest we either alter pg_settings so that we display value
*only* when source=default (set NULL otherwise) or we do some extra work
to derive what the setting would be if we ran RESET. The latter would be
preferred approach.

Since getting the value out when the source!=default is exactly the point
for the applications I was talking about, I'll rewrite the patch to expose
the reset_val as well and resubmit shortly.

Thanks for the feedback, your comments helped clarify the use-case for
reset_val a bit better for me and I'll be sure to document the two columns
appropriately. One perspective I don't get to see very often is that of a
regular user adjusting their settings.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD

#11Greg Smith
gsmith@gregsmith.com
In reply to: Simon Riggs (#2)
1 attachment(s)
Re: Add default_val to pg_settings

On Thu, 25 Sep 2008, Simon Riggs wrote:

So it would be useful to have a column that meant "if I run the RESET
command it would return me to this value".

Patch v3 attached that exposes boot_val and reset_val. The docs for the
latter link to the RESET command page for details.

Sample, with default_text_search_config snipped to fit the rest into a
reasonable width:

# select name,setting,reset_val,boot_val from pg_settings where
setting!=boot_val or setting!=reset_val;

name | setting | reset_val | boot_val
-----------------------+--------------------+--------------+---------
archive_command | (disabled) | |
client_encoding | UTF8 | UTF8 | SQL_ASCII
lc_collate | en_US.UTF-8 | en_US.UTF-8 | C
lc_ctype | en_US.UTF-8 | en_US.UTF-8 | C
lc_messages | en_US.UTF-8 | en_US.UTF-8 |
lc_monetary | en_US.UTF-8 | en_US.UTF-8 | C
lc_numeric | en_US.UTF-8 | en_US.UTF-8 | C
lc_time | en_US.UTF-8 | en_US.UTF-8 | C
log_timezone | US/Eastern | US/Eastern | UNKNOWN
max_fsm_pages | 204800 | 204800 | 20000
max_stack_depth | 2048 | 2048 | 100
server_encoding | UTF8 | UTF8 | SQL_ASCII
shared_buffers | 4096 | 4096 | 1024
TimeZone | US/Eastern | US/Eastern | UNKNOWN
timezone_abbreviations | Default | Default | UNKNOWN
transaction_isolation | read committed | default |

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD

Attachments:

guc-with-defaults-3.patchtext/plain; charset=US-ASCII; name=guc-with-defaults-3.patchDownload
Index: doc/src/sgml/catalogs.sgml
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/doc/src/sgml/catalogs.sgml,v
retrieving revision 2.174
diff -c -r2.174 catalogs.sgml
*** doc/src/sgml/catalogs.sgml	15 Sep 2008 18:43:41 -0000	2.174
--- doc/src/sgml/catalogs.sgml	6 Oct 2008 01:17:19 -0000
***************
*** 6422,6427 ****
--- 6422,6439 ----
        values)</entry>
       </row>
       <row>
+       <entry><structfield>boot_val</structfield></entry>
+       <entry><type>text</type></entry>
+       <entry>Parameter value assumed at server startup if the parameter is
+       not otherwise set</entry>
+      </row>
+      <row>
+       <entry><structfield>reset_val</structfield></entry>
+       <entry><type>text</type></entry>
+       <entry>Default run-time session value for the parameter that it will
+       revert to if <command>RESET</command></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
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.472
diff -c -r1.472 guc.c
*** src/backend/utils/misc/guc.c	10 Sep 2008 19:16:22 -0000	1.472
--- src/backend/utils/misc/guc.c	6 Oct 2008 01:20:47 -0000
***************
*** 6087,6092 ****
--- 6087,6094 ----
  	{
  		case PGC_BOOL:
  			{
+ 				struct config_bool *lconf = (struct config_bool *) conf;
+ 
  				/* min_val */
  				values[9] = NULL;
  
***************
*** 6095,6100 ****
--- 6097,6110 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+ 				/* boot_val */
+ 				snprintf(buffer, sizeof(buffer), "%s", lconf->boot_val ? "on" : "off");
+ 				values[12] = pstrdup(buffer);
+ 
+ 				/* reset_val */
+ 				snprintf(buffer, sizeof(buffer), "%s", lconf->reset_val ? "on" : "off");
+ 				values[13] = pstrdup(buffer);
  			}
  			break;
  
***************
*** 6112,6117 ****
--- 6122,6135 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+  				/* boot_val */
+  				snprintf(buffer, sizeof(buffer), "%d", lconf->boot_val);
+  				values[12] = pstrdup(buffer);
+ 
+  				/* reset_val */
+  				snprintf(buffer, sizeof(buffer), "%d", lconf->reset_val);
+  				values[13] = pstrdup(buffer);
  			}
  			break;
  
***************
*** 6129,6139 ****
--- 6147,6167 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+  				/* boot_val */
+  				snprintf(buffer, sizeof(buffer), "%g", lconf->boot_val);
+  				values[12] = pstrdup(buffer);
+ 
+  				/* reset_val */
+  				snprintf(buffer, sizeof(buffer), "%g", lconf->reset_val);
+  				values[13] = pstrdup(buffer);
  			}
  			break;
  
  		case PGC_STRING:
  			{
+  				struct config_string *lconf = (struct config_string *) conf;
+ 
  				/* min_val */
  				values[9] = NULL;
  
***************
*** 6142,6152 ****
--- 6170,6202 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+  				/* boot_val */
+  				if (lconf->boot_val == NULL)
+  				{
+  					values[12] = NULL;
+  				} else
+  				{
+  					snprintf(buffer, sizeof(buffer), "%s", lconf->boot_val);
+  					values[12] = pstrdup(buffer);
+  				}
+ 
+  				/* reset_val */
+  				if (lconf->reset_val == NULL)
+  				{
+  					values[13] = NULL;
+  				} else
+  				{
+  					snprintf(buffer, sizeof(buffer), "%s", lconf->reset_val);
+  					values[13] = pstrdup(buffer);
+  				}
  			}
  			break;
  
  		case PGC_ENUM:
  			{
+  				struct config_enum *lconf = (struct config_enum *) conf;
+ 
  				/* min_val */
  				values[9] = NULL;
  
***************
*** 6155,6160 ****
--- 6205,6220 ----
  
  				/* enumvals */
  				values[11] = config_enum_get_options((struct config_enum *) conf, "", "");
+ 
+  				/* boot_val */
+  				snprintf(buffer, sizeof(buffer), "%s", 
+ 					config_enum_lookup_by_value(lconf, lconf->boot_val));
+  				values[12] = pstrdup(buffer);
+ 
+  				/* reset_val */
+  				snprintf(buffer, sizeof(buffer), "%s", 
+ 					config_enum_lookup_by_value(lconf, lconf->reset_val));
+  				values[13] = pstrdup(buffer);
  			}
  			break;
  
***************
*** 6172,6177 ****
--- 6232,6243 ----
  
  				/* enumvals */
  				values[11] = NULL;
+ 
+  				/* boot_val */
+  				values[12] = NULL;
+ 
+  				/* reset_val */
+  				values[13] = NULL;
  			}
  			break;
  	}
***************
*** 6179,6192 ****
  	/* 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;
  	}
  }
  
--- 6245,6258 ----
  	/* If the setting came from a config file, set the source location */
  	if (conf->source == PGC_S_FILE)
  	{
! 		values[14] = conf->sourcefile;
  		snprintf(buffer, sizeof(buffer), "%d", conf->sourceline);
! 		values[15] = pstrdup(buffer);
  	}
  	else
  	{
! 		values[14] = NULL;
! 		values[15] = NULL;
  	}
  }
  
***************
*** 6223,6229 ****
   * 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)
--- 6289,6295 ----
   * show_all_settings - equiv to SHOW ALL command but implemented as
   * a Table Function.
   */
! #define NUM_PG_SETTINGS_ATTS	16
  
  Datum
  show_all_settings(PG_FUNCTION_ARGS)
***************
*** 6275,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);
  
  		/*
--- 6341,6353 ----
  						   TEXTOID, -1, 0);
  		TupleDescInitEntry(tupdesc, (AttrNumber) 12, "enumvals",
  						   TEXTOID, -1, 0);
!  		TupleDescInitEntry(tupdesc, (AttrNumber) 13, "boot_val",
!  						   TEXTOID, -1, 0);
!  		TupleDescInitEntry(tupdesc, (AttrNumber) 14, "reset_val",
!  						   TEXTOID, -1, 0);
! 		TupleDescInitEntry(tupdesc, (AttrNumber) 15, "sourcefile",
  						   TEXTOID, -1, 0);
! 		TupleDescInitEntry(tupdesc, (AttrNumber) 16, "sourceline",
  						   INT4OID, -1, 0);
  
  		/*
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.514
diff -c -r1.514 pg_proc.h
*** src/include/catalog/pg_proc.h	10 Sep 2008 18:09:20 -0000	1.514
--- src/include/catalog/pg_proc.h	6 Oct 2008 01:07:17 -0000
***************
*** 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");
--- 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,25,25,23}" "{o,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,boot_val,reset_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/test/regress/expected/rules.out
===================================================================
RCS file: /home/gsmith/cvsrepo/pgsql/src/test/regress/expected/rules.out,v
retrieving revision 1.142
diff -c -r1.142 rules.out
*** src/test/regress/expected/rules.out	10 Sep 2008 18:09:20 -0000	1.142
--- src/test/regress/expected/rules.out	6 Oct 2008 01:07:53 -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, 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"]));
--- 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.boot_val, a.reset_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"]));
#12Magnus Hagander
magnus@hagander.net
In reply to: Greg Smith (#11)
Re: Add default_val to pg_settings

Greg Smith wrote:

On Thu, 25 Sep 2008, Simon Riggs wrote:

So it would be useful to have a column that meant "if I run the RESET
command it would return me to this value".

Patch v3 attached that exposes boot_val and reset_val. The docs for the
latter link to the RESET command page for details.

Applied with some smaller changes and minor stylistic fixes. You don't
need to copy a text string into buffer and then pstrdup() buffer later -
you can just pstrdup() the text string directly. And you forgot one of
the places in rules.out :-P

Thanks!

//Magnus

#13Decibel!
decibel@decibel.org
In reply to: Greg Smith (#11)
1 attachment(s)
Re: Add default_val to pg_settings

On Oct 5, 2008, at 8:50 PM, Greg Smith wrote:

Patch v3 attached that exposes boot_val and reset_val. The docs
for the latter link to the RESET command page for details.

<nitpick>Is it really that important that we save 2 characters on
each field name?</nitpick>
--
Decibel!, aka Jim C. Nasby, Database Architect decibel@decibel.org
Give your computer some brain candy! www.distributed.net Team #1828

Attachments:

smime.p7sapplication/pkcs7-signature; name=smime.p7sDownload