Subject: [PATCH] Add fileval-bootval consistency check of GUC parameters

From: Kyotaro Horiguchi <[email-masked]>
Date: Thu, 26 May 2022 15:55:34 +0900

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

----Next_Part(Thu_May_26_16_27_53_2022_916)----