At Thu, 16 Jun 2022 08:23:07 -0500, Justin Pryzby <pry...@telsasoft.com> wrote
in
> On Thu, Jun 16, 2022 at 05:19:46PM +0900, Kyotaro Horiguchi wrote:
> > At Sat, 11 Jun 2022 09:41:37 -0500, Justin Pryzby <pry...@telsasoft.com>
> > wrote in
> > > Note that this gives:
> > >
> > > guc.c:7573:9: warning: ‘dst’ may be used uninitialized in this function
> > > [-Wmaybe-uninitialized]
> >
> > Mmm. I don't have an idea where the 'dst' came from...
>
> Well, in your latest patch, you've renamed it.
>
> guc.c:7586:19: warning: ‘result’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
> 7586 | PG_RETURN_TEXT_P(cstring_to_text(result));
Ooo. I find that the patch on my hand was different from that on this
list by some reason uncertain to me. I now understand what's
happening.
At Sat, 11 Jun 2022 09:41:37 -0500, Justin Pryzby <pry...@telsasoft.com> wrote
in
> with gcc version 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2)
My compiler (gcc 8.5.0) (with -Wswitch) is satisfied by finding that
the switch() covers all enum values. I don't know why the new
compiler complains with this, but compilers in such environment should
shut up by the following change.
- char *result;
+ char *result = "";
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From d67c13712ab1d47956c1c311d7ed80342fa51e2f Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Thu, 16 Jun 2022 17:08:02 +0900
Subject: [PATCH v3] 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/backend/utils/misc/guc.c | 53 ++++++++++++++++
src/include/catalog/pg_proc.dat | 5 ++
src/test/modules/test_misc/t/003_check_guc.pl | 60 +++++++++++++++++--
3 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a7cc49898b..136a21ba88 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7520,6 +7520,59 @@ parse_and_validate_value(struct config_generic *record,
return true;
}
+/*
+ * Convert value to unitless value according to the specified GUC variable
+ */
+Datum
+pg_config_unitless_value(PG_FUNCTION_ARGS)
+{
+ char *name = "";
+ char *value = "";
+ struct config_generic *record;
+ char *result = "";
+ void *extra;
+ union config_var_val val;
+ const char *p;
+ char buffer[256];
+
+ if (!PG_ARGISNULL(0))
+ name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ if (!PG_ARGISNULL(1))
+ value = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+ record = find_option(name, true, false, ERROR);
+
+ parse_and_validate_value(record, name, value, PGC_S_TEST, WARNING,
+ &val, &extra);
+
+ switch (record->vartype)
+ {
+ case PGC_BOOL:
+ result = (val.boolval ? "on" : "off");
+ break;
+ case PGC_INT:
+ snprintf(buffer, sizeof(buffer), "%d", val.intval);
+ result = pstrdup(buffer);
+ break;
+ case PGC_REAL:
+ snprintf(buffer, sizeof(buffer), "%g", val.realval);
+ result = pstrdup(buffer);
+ break;
+ case PGC_STRING:
+ p = val.stringval;
+ if (p == NULL)
+ p = "";
+ result = pstrdup(p);
+ break;
+ case PGC_ENUM:
+ p = config_enum_lookup_by_value((struct config_enum *)record,
+ val.boolval);
+ result = pstrdup(p);
+ break;
+ }
+
+ PG_RETURN_TEXT_P(cstring_to_text(result));
+}
/*
* Sets option `name' to given value.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 87aa571a33..9eb2a584e1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6118,6 +6118,11 @@
proname => 'pg_settings_get_flags', provolatile => 's', prorettype => '_text',
proargtypes => 'text', prosrc => 'pg_settings_get_flags' },
+{ oid => '9956', descr => 'normalize value to the unit of specified GUC',
+ proname => 'pg_config_unitless_value', proisstrict => 'f',
+ provolatile => 's', prorettype => 'text', proargtypes => 'text text',
+ proargnames => '{varname,value}', prosrc => 'pg_config_unitless_value' },
+
{ oid => '3329', descr => 'show config file settings',
proname => 'pg_show_all_file_settings', prorows => '1000', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
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..9de6a386de 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,41 @@ my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
$node->start;
+# 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',
+ 'wal_buffers'
+ );
+
# 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 lower(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", $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(
@@ -43,7 +66,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 @check_elems = ();
open(my $contents, '<', $sample_file)
|| die "Could not open $sample_file: $!";
while (my $line = <$contents>)
@@ -53,11 +76,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 +94,39 @@ 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.
+ if (!grep { $_ eq $param_name } @ignored_parameters)
+ {
+ push (@check_elems, "('$param_name','$file_value')");
+ }
}
}
close $contents;
+# Run consistency check between config-file's default value and boot
+# values. To show sample setting that is not found in the view, use
+# LEFT JOIN and make sure pg_settings.name is not NULL.
+my $check_query =
+ 'SELECT f.n, f.v, s.boot_val FROM (VALUES '.
+ join(',', @check_elems).
+ ') f(n,v) LEFT JOIN pg_settings s ON lower(s.name) = f.n '.
+ "WHERE pg_config_unitless_value(f.n, f.v) <> COALESCE(s.boot_val, '') ".
+ 'OR s.name IS NULL';
+
+print $check_query;
+
+is ($node->safe_psql('postgres', $check_query), '',
+ 'check if fileval-bootval consistency is fine');
+
# 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");
--
2.31.1
