From: | Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | David Steele <david(at)pgmasters(dot)net> |
Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>, David Fetter <david(at)fetter(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Proposal: knowing detail of config files via SQL |
Date: | 2015-04-28 02:37:23 |
Message-ID: | CAD21AoAH7Lpk2ncP024L1rSm2MO_qP+S0eJ27nA=xQbZXWLV1g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Apr 28, 2015 at 3:22 AM, David Steele <david(at)pgmasters(dot)net> wrote:
> On 4/27/15 10:31 AM, Sawada Masahiko wrote:
>> Thank you for your review comment!
>> The latest patch is attached.
>
> Looks good overall - a few more comments below:
Thank you for your reviewing.
Attached v8 patch is latest version.
> diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
> + <row>
> + <entry><structfield>seqno</structfield></entry>
> + <entry><structfield>integer</structfield></entry>
> + <entry>Sequence number of current view</entry>
>
> I would suggest:
>
> Order in which the setting was loaded from the configuration
FIx.
> + </row>
> + <row>
> + <entry><structfield>name</structfield></entry>
> + <entry><structfield>text</structfield></entry>
>
> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> +/*
> + * show_all_file_settings
> + */
> +
> +#define NUM_PG_FILE_SETTINGS_ATTS 5
> +
> +Datum
> +show_all_file_settings(PG_FUNCTION_ARGS)
>
> It would be good to get more detail in the function comment, as well as
> more comments in the function itself.
Added some comments.
> A minor thing, but there are a number of whitespace errors when applying
> the patch:
>
> ../000_pg_file_settings_view_v6.patch:159: indent with spaces.
> free(guc_file_variables[i].name);
> ../000_pg_file_settings_view_v6.patch:160: indent with spaces.
> free(guc_file_variables[i].value);
> ../000_pg_file_settings_view_v6.patch:161: indent with spaces.
> free(guc_file_variables[i].filename);
> ../000_pg_file_settings_view_v6.patch:178: indent with spaces.
> guc_array->name = guc_strdup(FATAL, item->name);
> ../000_pg_file_settings_view_v6.patch:179: indent with spaces.
> guc_array->value = guc_strdup(FATAL, item->value);
> warning: squelched 2 whitespace errors
> warning: 7 lines add whitespace errors.
>
> I'm sure the committer would appreciate it if you'd clean those up.
I tried to get rid of white space.
Regards,
-------
Sawada Masahiko
Attachment | Content-Type | Size |
---|---|---|
000_pg_file_settings_view_v8.patch | text/x-diff | 11.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2015-04-28 02:49:34 | Re: Temporal extensions |
Previous Message | Jim Nasby | 2015-04-28 02:35:58 | Re: Allow SQL/plpgsql functions to accept record |