Re: Proposal: knowing detail of config files via SQL

From: David Steele <david(at)pgmasters(dot)net>
To: Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com>
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-27 18:22:36
Message-ID: 553E7E6C.2060401@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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:

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

+ </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.

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.

--
- David Steele
david(at)pgmasters(dot)net

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2015-04-27 20:19:12 Re: INSERT ... ON CONFLICT syntax issues
Previous Message Stephen Frost 2015-04-27 18:01:52 Re: GSSAPI, SSPI - include_realm default