At Thu, 26 May 2022 13:00:45 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
> At Wed, 25 May 2022 21:25:53 -0500, Justin Pryzby <pry...@telsasoft.com>
> wrote in
> > And I think these should be updated it postgresql.conf to use the same unit
> > as
> > in current_setting().
> >
> > track_activity_query_size | 1kB
> > | 1024
> > wal_buffers | 4MB
> > | -1
> > wal_receiver_timeout | 1min
> > | 60s
> > wal_sender_timeout | 1min
> > | 60s
>
> I'm not sure we should do so. Rather I'd prefer 60s than 1min here.
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.
So the attached is a wip version of that
Numeric values are compared considering units. But does not require
the units of the both values to match. Some variables are ignored by
an explicit instruction (ignored_parameters). Some variables are
compared case-insensitively by an explicit instruction
(case_insensitive_params). bool and enum are compared
case-insensitively automatically.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From d06e248ced6a45aa10badae9b223e016605e61c5 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Thu, 26 May 2022 15:55:34 +0900
Subject: [PATCH] Add fileval-bootval consistency check of GUC parameters
We should keep GUC variables consistent between the default values
written in postgresql.conf.sample and defined in guc.c. Add an
automated way to check for the consistency to the TAP test suite. Some
variables are still excluded since they cannot be checked simple way.
---
src/test/modules/test_misc/t/003_check_guc.pl | 151 +++++++++++++++++-
1 file changed, 146 insertions(+), 5 deletions(-)
diff --git a/src/test/modules/test_misc/t/003_check_guc.pl b/src/test/modules/test_misc/t/003_check_guc.pl
index 60459ef759..efaa7ec182 100644
--- a/src/test/modules/test_misc/t/003_check_guc.pl
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -11,18 +11,65 @@ my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
$node->start;
+# unit conversion factors
+my %unit_conv_factor =
+ ('b', 1,
+ 'kb', 1024,
+ 'mb', 1024 * 1024,
+ 'gb', 1024 * 1024 * 1024,
+ 'tb', 1024 * 1024 * 1024 * 1024,
+ '8kb', 8 * 1024,
+ 'us', 1.0 / (1000 / 1000),
+ 'ms', 1.0 / 1000,
+ 's', 1,
+ 'min', 60,
+ 'h', 3600,
+ 'd', 24 * 3600);
+
+# parameter names that cannot get consistency check performed
+my @ignored_parameters =
+ ('data_directory',
+ 'hba_file',
+ 'ident_file',
+ 'krb_server_keyfile',
+ 'max_stack_depth',
+ 'bgwriter_flush_after',
+ 'wal_sync_method',
+ 'checkpoint_flush_after',
+ 'timezone_abbreviations',
+ 'lc_messages');
+
+# 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');
+
# Grab the names of all the parameters that can be listed in the
# configuration sample file. config_file is an exception, it is not
# in postgresql.conf.sample but is part of the lists from guc.c.
my $all_params = $node->safe_psql(
'postgres',
- "SELECT name
+ "SELECT name, vartype, unit, boot_val, '!'
FROM pg_settings
WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
name <> 'config_file'
ORDER BY 1");
# Note the lower-case conversion, for consistency.
-my @all_params_array = split("\n", lc($all_params));
+my %all_params_hash;
+foreach my $line (split("\n", lc($all_params)))
+{
+ my @f = split('\|', $line);
+ fail("query returned wrong number of columns: $#f : $line") if ($#f != 4);
+ $all_params_hash{$f[0]}->{type} = $f[1];
+ $all_params_hash{$f[0]}->{unit} = $f[2];
+ $all_params_hash{$f[0]}->{bootval} = $f[3];
+}
# Grab the names of all parameters marked as NOT_IN_SAMPLE.
my $not_in_sample = $node->safe_psql(
@@ -44,6 +91,7 @@ my @gucs_in_file;
# Read the sample file line-by-line, checking its contents to build a list
# of everything known as a GUC.
my $num_tests = 0;
+my $failed = 0;
open(my $contents, '<', $sample_file)
|| die "Could not open $sample_file: $!";
while (my $line = <$contents>)
@@ -53,11 +101,16 @@ while (my $line = <$contents>)
# file.
# - Valid configuration options are followed immediately by " = ",
# with one space before and after the equal sign.
- if ($line =~ m/^#?([_[:alpha:]]+) = .*/)
+ if ($line =~ m/^#?([_[:alpha:]]+) = (.*)$/)
{
# Lower-case conversion matters for some of the GUCs.
my $param_name = lc($1);
+ # extract value
+ my $file_value = $2;
+ $file_value =~ s/\s*#.*$//; # strip trailing comment
+ $file_value =~ s/^'(.*)'$/$1/; # strip quotes
+
# Ignore some exceptions.
next if $param_name eq "include";
next if $param_name eq "include_dir";
@@ -66,19 +119,28 @@ while (my $line = <$contents>)
# Update the list of GUCs found in the sample file, for the
# follow-up tests.
push @gucs_in_file, $param_name;
+
+ # Check for consistency between bootval and file value.
+ $failed++
+ if (!check_val($param_name,
+ $all_params_hash{$param_name}->{type},
+ $all_params_hash{$param_name}->{bootval},
+ $all_params_hash{$param_name}->{unit},
+ $file_value));
}
}
close $contents;
+pass("fileval-bootval consistency is fine") if ($failed == 0);
+
# Cross-check that all the GUCs found in the sample file match the ones
# fetched above. This maps the arrays to a hash, making the creation of
# each exclude and intersection list easier.
my %gucs_in_file_hash = map { $_ => 1 } @gucs_in_file;
-my %all_params_hash = map { $_ => 1 } @all_params_array;
my %not_in_sample_hash = map { $_ => 1 } @not_in_sample_array;
-my @missing_from_file = grep(!$gucs_in_file_hash{$_}, @all_params_array);
+my @missing_from_file = grep(!$gucs_in_file_hash{$_}, keys %all_params_hash);
is(scalar(@missing_from_file),
0, "no parameters missing from postgresql.conf.sample");
@@ -108,3 +170,82 @@ foreach my $param (@sample_intersect)
}
done_testing();
+
+
+# parse $val according to $type.
+# $val may be suffixed by a unit if $optunit is not given.
+sub parse_val
+{
+ my ($type, $val, $optunit) = @_;
+
+ if ($val =~ /^(\d+)([a-zA-Z]*)$/)
+ {
+ $val = $1;
+ my $unit = $2;
+
+ # integer allows octal representaion
+ $val = oct($val) if ($type eq "integer" && $val =~ /^0./);
+
+ # accept $optunit only if $val was not suffixed by a unit
+ if (length $optunit)
+ {
+ fail("check if unit \"$unit\" is known to this script")
+ if (length $unit);
+
+ $unit = $optunit;
+ }
+
+
+ if (length($unit))
+ {
+ $unit = lc($unit);
+
+ fail("check if unit \"$unit\" is known to this script")
+ if (!defined $unit_conv_factor{$unit});
+
+ $val *= $unit_conv_factor{$unit};
+ }
+ }
+
+ return $val;
+}
+
+# check if $bootval and $fileval match according to $type and $unit
+sub check_val
+{
+ my ($param_name, $type, $bootval, $unit, $fileval) = @_;
+
+ my $match = 0;
+
+ if (grep { $_ eq $param_name } @ignored_parameters)
+ {
+ # this parameter cannot be checked, skip.
+ $match = 1;
+ }
+ elsif ($type eq "integer" || $type eq "real")
+ {
+ # this parameter is numeric, parse with unit conversion
+ my $bootval = parse_val($type, $bootval, $unit);
+ my $fileval = parse_val($type, $fileval);
+
+ $match = 1 if ($bootval == $fileval);
+ }
+ elsif (($type eq "bool" || $type eq "enum") ||
+ grep { $_ eq $param_name} @case_insensitive_params)
+ {
+ # this parameter is compared case-insensitively
+ $match = 1 if (lc($bootval) eq lc($fileval));
+ }
+ else
+ {
+ # exact match
+ $match = 1 if ($bootval eq $fileval);
+ }
+
+ if (!$match)
+ {
+ fail("$param_name inconsistent (\"$bootval\" vs \"$fileval\")");
+ }
+
+ return $match;
+}
--
2.31.1
