On Thu, Mar 5, 2015 at 12:04 PM, Peter Eisentraut <[email protected]> wrote:
> On 2/17/15 10:45 AM, Robert Haas wrote:
>> You don't really need the "else" here, and in parallel cases:
>>
>> if (*conf->variable != newval)
>> {
>> + record->status |= GUC_PENDING_RESTART;
>> ereport(elevel,
>> (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
>> errmsg("parameter \"%s\" cannot be
>> changed without restarting the server",
>> name)));
>> return 0;
>> }
>> + else
>> + record->status &= ~GUC_PENDING_RESTART;
>> return -1;
>>
>> The if-statement ends with "return 0" so there is no reason for the "else".
>
> I kind of liked the symmetry of if/else, but I can change it.
This feature looks useful to me. I had a quick look and it is working
as intended: issuing SIGHUP to reload parameters updates the
pending_restart status correctly.
One additional comment on top of what has already been mentioned is
that this lacks parenthesis IMO:
- values[16] = conf->status & GUC_PENDING_RESTART ? "t" : "f";
+ values[16] = (conf->status & GUC_PENDING_RESTART) ? "t" : "f";
Also, documentation was not correctly formatted.
Changes with ALTER SYSTEM (and include files) get recognized as well.
For example:
=# \! echo max_prepared_transactions = 100 >> $PGDATA/postgresql.conf
=# select pg_reload_conf();
pg_reload_conf
----------------
t
(1 row)
=# select name from pg_settings where pending_restart;
name
---------------------------
max_prepared_transactions
(1 row)
=# alter system set max_connections = 1000;
ALTER SYSTEM
=# select pg_reload_conf();
pg_reload_conf
----------------
t
(1 row)
=# select name from pg_settings where pending_restart;
name
---------------------------
max_connections
max_prepared_transactions
(2 rows)
Attached is a rebased patch with previous comments addressed as I was
looking at it.
Switching this patch as "Ready for committer".
Regards,
--
Michael
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index d0b78f2..53d3f4f 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -8822,6 +8822,14 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
or when examined by a non-superuser)
</entry>
</row>
+ <row>
+ <entry><structfield>pending_restart</structfield></entry>
+ <entry><type>boolean</type></entry>
+ <entry><literal>true</literal> if the value has been changed in the
+ configuration file but needs a restart; or <literal>false</literal>
+ otherwise.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f43aff2..e09b021 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5897,12 +5897,14 @@ set_config_option(const char *name, const char *value,
{
if (*conf->variable != newval)
{
+ record->status |= GUC_PENDING_RESTART;
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed without restarting the server",
name)));
return 0;
}
+ record->status &= ~GUC_PENDING_RESTART;
return -1;
}
@@ -5985,12 +5987,14 @@ set_config_option(const char *name, const char *value,
{
if (*conf->variable != newval)
{
+ record->status |= GUC_PENDING_RESTART;
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed without restarting the server",
name)));
return 0;
}
+ record->status &= ~GUC_PENDING_RESTART;
return -1;
}
@@ -6073,12 +6077,14 @@ set_config_option(const char *name, const char *value,
{
if (*conf->variable != newval)
{
+ record->status |= GUC_PENDING_RESTART;
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed without restarting the server",
name)));
return 0;
}
+ record->status &= ~GUC_PENDING_RESTART;
return -1;
}
@@ -6179,12 +6185,14 @@ set_config_option(const char *name, const char *value,
if (*conf->variable == NULL || newval == NULL ||
strcmp(*conf->variable, newval) != 0)
{
+ record->status |= GUC_PENDING_RESTART;
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed without restarting the server",
name)));
return 0;
}
+ record->status &= ~GUC_PENDING_RESTART;
return -1;
}
@@ -6272,12 +6280,14 @@ set_config_option(const char *name, const char *value,
{
if (*conf->variable != newval)
{
+ record->status |= GUC_PENDING_RESTART;
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
errmsg("parameter \"%s\" cannot be changed without restarting the server",
name)));
return 0;
}
+ record->status &= ~GUC_PENDING_RESTART;
return -1;
}
@@ -7970,6 +7980,8 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow)
values[14] = NULL;
values[15] = NULL;
}
+
+ values[16] = (conf->status & GUC_PENDING_RESTART) ? "t" : "f";
}
/*
@@ -8005,7 +8017,7 @@ show_config_by_name(PG_FUNCTION_ARGS)
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
-#define NUM_PG_SETTINGS_ATTS 16
+#define NUM_PG_SETTINGS_ATTS 17
Datum
show_all_settings(PG_FUNCTION_ARGS)
@@ -8065,6 +8077,8 @@ show_all_settings(PG_FUNCTION_ARGS)
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 16, "sourceline",
INT4OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 17, "pending_restart",
+ BOOLOID, -1, 0);
/*
* Generate attribute metadata needed later to produce tuples from raw
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 619d996..46879b3 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3048,7 +3048,7 @@ DATA(insert OID = 2077 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 1
DESCR("SHOW X as a function");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 0 f f f f f f v 3 0 25 "25 25 16" _null_ _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 0 f f f f t t s 0 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,1009,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}" _null_ show_all_settings _null_ _null_ _null_ ));
+DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t s 0 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,1009,25,25,25,23,16}" "{o,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,pending_restart}" _null_ 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 0 f f f f t t v 0 0 2249 "" "{25,26,26,23,21,25,28,26,26,21,25,23,25,16,16}" "{o,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,fastpath}" _null_ pg_lock_status _null_ _null_ _null_ ));
DESCR("view system lock information");
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index cf319af..c0f9cb9 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -167,6 +167,7 @@ struct config_generic
* Caution: the GUC_IS_IN_FILE bit is transient state for ProcessConfigFile.
* Do not assume that its value represents useful information elsewhere.
*/
+#define GUC_PENDING_RESTART 0x0002
/* GUC records for specific variable types */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 71fa44a..8590834 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1600,8 +1600,9 @@ pg_settings| SELECT a.name,
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, boot_val, reset_val, sourcefile, sourceline);
+ a.sourceline,
+ a.pending_restart
+ FROM pg_show_all_settings() a(name, setting, unit, category, short_desc, extra_desc, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, sourcefile, sourceline, pending_restart);
pg_shadow| SELECT pg_authid.rolname AS usename,
pg_authid.oid AS usesysid,
pg_authid.rolcreatedb AS usecreatedb,
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
