On 4/4/15 9:21 AM, Sawada Masahiko wrote: > I added documentation changes to patch is attached. > Also I tried to use memory context for allocation of guc_file_variable > in TopMemoryContext, > but it was failed access after received SIGHUP. Below is my review of the v5 patch:
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
+ <sect1 id="view-pg-file-settings">
+ <title><structname>pg_file_settings</structname></title>
+
+ <indexterm zone="view-pg-file-settings">
+ <primary>pg_file_settings</primary>
+ </indexterm>
+
+ <para>
+ The view <structname>pg_file_settings</structname> provides confirm
parameters via SQL,
+ which is written in configuration file. This view can be update by
reloading configration file.
+ </para>
Perhaps something like:
<para>
The view <structname>pg_file_settings</structname> provides access to
run-time parameters
that are defined in configuration files via SQL. In contrast to
<structname>pg_settings</structname> a row is provided for each
occurrence
of the parameter in a configuration file. This is helpful for
discovering why one value
may have been used in preference to another when the parameters were
loaded.
</para>
+ <table>
+ <title><structname>pg_file_settings</> Columns</title>
+
+ <tgroup cols="3">
+ <thead>
+ <row>
+ <entry>Name</entry>
+ <entry>Type</entry>
+ <entry>Description</entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><structfield>sourcefile</structfield></entry>
+ <entry><structfield>text</structfield></entry>
+ <entry>A path of configration file</entry>
From pg_settings:
<entry>Configuration file the current value was set in (null for
values set from sources other than configuration files, or when
examined by a non-superuser);
helpful when using <literal>include</> directives in configuration
files</entry>
+ </row>
+ <row>
+ <entry><structfield>sourceline</structfield></entry>
+ <entry><structfield>integer</structfield></entry>
+ <entry>The line number in configuration file</entry>
From pg_settings:
<entry>Line number within the configuration file the current value was
set at (null for values set from sources other than configuration
files,
or when examined by a non-superuser)
</entry>
+ </row>
+ <row>
+ <entry><structfield>name</structfield></entry>
+ <entry><structfield>text</structfield></entry>
+ <entry>Parameter name in configuration file</entry>
From pg_settings:
<entry>Run-time configuration parameter name</entry>
+ </row>
+ <row>
+ <entry><structfield>setting</structfield></entry>
+ <entry><structfield>text</structfield></entry>
+ <entry>Value of the parameter in configuration file</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+</sect1>
diff --git a/src/backend/utils/misc/guc-file.l
b/src/backend/utils/misc/guc-file.l
@@ -342,6 +345,42 @@ ProcessConfigFile(GucContext context)
+ guc_array = guc_file_variables;
+ for (item = head; item; item = item->next, guc_array++)
+ {
+ free(guc_array->name);
+ free(guc_array->value);
+ free(guc_array->filename);
There's an issue freeing these when calling pg_reload_conf():
postgres=# alter system set max_connections = 100;
ALTER SYSTEM
postgres=# select * from pg_reload_conf();
LOG: received SIGHUP, reloading configuration files
pg_reload_conf
----------------
t
(1 row)
postgres=# postgres(25078,0x7fff747b2300) malloc: *** error for object
0x424d380044: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Of course a config reload can't change max_connections, but I wouldn't
expect it to crash, either.
+ guc_array->sourceline = -1;
I can't see the need for this since it is reassigned further down.
--
Up-thread David J had recommended an ordering column (or seqno) that
would provide the order in which the settings were loaded. I think this
would give valuable information that can't be gleaned from the line
numbers alone.
Personally I think it would be fine to start from 1 and increment for
each setting found, rather than rank within a setting. If the user
wants per setting ranking that's what SQL is for. So the output would
look something like:
sourcefile | sourceline | seqno | name | setting
--------------------------+------------+-------+-----------------+-----------
/db/postgresql.conf | 64 | 1 | max_connections | 100
/db/postgresql.conf | 116 | 2 | shared_buffers | 128MB
/db/postgresql.conf | 446 | 3 | log_timezone |
US/Eastern
/db/postgresql.auto.conf | 3 | 4 | max_connections | 200
--
- David Steele
da...@pgmasters.net
signature.asc
Description: OpenPGP digital signature
