Date: Mon, 30 May 2022 17:27:19 +0900 (JST)
Message-Id: <20220530.172719.867839911471994005.horikyota.ntt@gmail.com>
To: andres@anarazel.de
Cc: pryzby@telsasoft.com, michael@paquier.xyz, nathandbossart@gmail.com,
 pgsql-hackers@postgresql.org
Subject: Re: fix stats_fetch_consistency value in postgresql.conf.sample
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20220528202245.lrjn3v2m5shzgbsq@alap3.anarazel.de>
References: <20220526.130045.1582015671130082847.horikyota.ntt@gmail.com>
	<20220526.162753.632795261813283794.horikyota.ntt@gmail.com>
	<20220528202245.lrjn3v2m5shzgbsq@alap3.anarazel.de>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Mon_May_30_17_27_19_2022_009)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20220530.172719.867839911471994005.horikyota.ntt%40gmail.com>
Precedence: bulk

----Next_Part(Mon_May_30_17_27_19_2022_009)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

At Sat, 28 May 2022 13:22:45 -0700, Andres Freund <andres@anarazel.de> wrote in 
> Hi,
> 
> On 2022-05-26 16:27:53 +0900, Kyotaro Horiguchi wrote:
> > It could be in SQL, but *I* prefer to use perl for this, since it
> > allows me to write a bit complex things (than simple string
> > comparison) simpler.
> 
> I wonder if we shouldn't just expose a C function to do this, rather than
> having a separate implementation in a tap test.

It was annoying that I needed to copy the unit-conversion stuff.  I
did that in the attached.  parse_val() and check_val() and the duped
data is removed.

> > +# parameter names that cannot get consistency check performed
> > +my @ignored_parameters =
> 
> I think most of these we could ignore by relying on source <> 'override'
> instead of listing them?
> 
> 
> > +# parameter names that requires case-insensitive check
> > +my @case_insensitive_params =
> > +  ('ssl_ciphers',
> > +   'log_filename',
> > +   'event_source',
> > +   'log_timezone',
> > +   'timezone',
> > +   'lc_monetary',
> > +   'lc_numeric',
> > +   'lc_time');
> 
> Why do these differ by case?

Mmm. It just came out of a thinko. I somehow believed that the script
down-cases only the parameter names among the values from
pg_settings. I felt that something's strange while on it,
though.. After fixing it, there are only the following values that
differ only in letter cases. In passing I changed "bool" and "enum"
are case-sensitive, too.

name              conf          bootval
client_encoding: "sql_ascii"  "SQL_ASCII"
datestyle      : "iso, mdy"   "ISO, MDY"
syslog_facility: "LOCAL0"     "local0"

It seems to me that the bootval is right for all variables.


I added a testing-aid function pg_normalize_config_option(name,value)
so the consistency check can be performed like this.

SELECT f.n, f.v, s.boot_val
    FROM (VALUES ('work_mem','4MB'),...) f(n,v)
    JOIN pg_settings s ON s.name = f.n '.
    WHERE pg_normalize_config_value(f.n, f.v) <> '.
          pg_normalize_config_value(f.n, s.boot_val)';

There're some concerns on the function.

- _ShowConfig() returns archive_command as "(disabled)" regardless of
  its value.  The test passes accidentally for the variable...

- _ShowConfig() errors out for "timezone_abbreviations" and "" since
  the check function tries to open the timezone file. (It is excluded
  from the test.)

I don't want to create a copy of the function only for this purpose.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

----Next_Part(Mon_May_30_17_27_19_2022_009)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="v2-0001-Add-fileval-bootval-consistency-check-of-GUC-para.patch"