Re: Proposal: knowing detail of config files via SQL

On Mon, Apr 27, 2015 at 11:31 PM, Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Sat, Apr 25, 2015 at 3:40 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>> 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:
>
> Thank you for your review comment!
> The latest patch is attached.
>
>> 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>
>
> The documentation is updated based on your suggestion.
>
>> 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():
>
> Fix.
>
>> 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.
>
> Fix.
>
>> --
>>
>> 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
>>
>
> Yep, I agree with you.
> Added seqno column into pg_file_settings view.
>

Attached patch is modified to apply to HEAD.
v7 patch is latest.

Regards,

-------
Sawada Masahiko