Re: fix stats_fetch_consistency value in postgresql.conf.sample

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: andres(at)anarazel(dot)de
Cc: pryzby(at)telsasoft(dot)com, michael(at)paquier(dot)xyz, nathandbossart(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: fix stats_fetch_consistency value in postgresql.conf.sample
Date: 2022-05-30 08:27:19
Message-ID: 20220530.172719.867839911471994005.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Sat, 28 May 2022 13:22:45 -0700, Andres Freund <andres(at)anarazel(dot)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

Attachment Content-Type Size
v2-0001-Add-fileval-bootval-consistency-check-of-GUC-para.patch text/x-patch 8.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2022-05-30 08:37:40 ParseTzFile doesn't FreeFile on error
Previous Message Peter Smith 2022-05-30 08:21:36 Re: Skipping schema changes in publication