At Thu, 2 Mar 2023 11:54:10 +0100, Peter Eisentraut
<peter.eisentr...@enterprisedb.com> wrote in
> On 27.02.23 09:32, Kyotaro Horiguchi wrote:
> > I found it frustrating that the line "shared_buffers = 0.1GB" in
> > postgresql.conf postgresql.conf was causing an error and that the
> > value required (additional) surrounding single quotes. The attached
> > patch makes the parser accept the use of non-quoted real values
> > followed by a unit for such variables. I'm not sure if that syntax
> > fully covers the input syntax of strtod, but I beieve it is suffucient
> > for most use cases.
>
> This seems sensible to fix. If you're not sure about the details,
> write some test cases. :)
Thanks. I initially intended to limit the change for REAL to accept
units following a value. However, actually I also modified it to
accept '1e1'.
man strtod says it accepts the following format.
> The expected form of the (initial portion of the) string is optional
> leading white space as recognized by isspace(3), an optional plus ('+')
> or minus sign ('-') and then either (i) a decimal number, or (ii) a
> hexadecimal number, or (iii) an infinity, or (iv) a NAN (not-a-number).
>
> A decimal number consists of a nonempty sequence of decimal digits pos‐
> sibly containing a radix character (decimal point, locale-dependent,
> usually '.'), optionally followed by a decimal exponent. A decimal
> exponent consists of an 'E' or 'e', followed by an optional plus or
> minus sign, followed by a nonempty sequence of decimal digits, and
> indicates multiplication by a power of 10.
It is written in regexp as
'\s*[-+]?(\.\d+|\d+(\.\d*)?)([Ee][-+]?\d+)?'. The leading whitespace
is unnecessary in this specific usage, and we also need to exclude
INTERGER from this set of matches. Therefore, it should be modified to
'[-+]?((\.\d+|\d+\.\d*)([Ee][-+]?\d+)?|\d+([Ee][-+]?\d+))'.
It is translated to the following BNF notation. UNIT_LETTER is also
needed.
{SIGN}?(("."{DIGIT}+|{DIGIT}+"."{DIGIT}*){EXPONENT}?|{DIGIT}+{EXPONENT}){UNIT_LETTER}*
The attached patch applies aforementioned change to guc-file.l and
includes tests for values in certain patters.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 9a413b791364152d44838d329f0efe3cc7db4d22 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Fri, 3 Mar 2023 13:39:26 +0900
Subject: [PATCH v1] Widen unquoted value acceptance in configuration file
Numeric configuration parameters can accept real numbers like '10.3',
but the values cannot be followed by a unit if they are not quoted. In
addition, certain formats like '1e4' are not accepted unless they are
not quoted. This commit changes the configuration file parser to
accept all of these values without the need for quotes.
---
src/backend/utils/misc/guc-file.l | 2 +-
src/test/modules/test_misc/t/003_check_guc.pl | 41 +++++++++++++++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 41d62a9f23..ca4add9710 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -76,7 +76,7 @@ UNIT_LETTER [a-zA-Z]
INTEGER {SIGN}?({DIGIT}+|0x{HEXDIGIT}+){UNIT_LETTER}*
EXPONENT [Ee]{SIGN}?{DIGIT}+
-REAL {SIGN}?{DIGIT}*"."{DIGIT}*{EXPONENT}?
+REAL {SIGN}?(("."{DIGIT}+|{DIGIT}+"."{DIGIT}*){EXPONENT}?|{DIGIT}+{EXPONENT}){UNIT_LETTER}*
LETTER [A-Za-z_\200-\377]
LETTER_OR_DIGIT [A-Za-z_0-9\200-\377]
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 e9f33f3c77..053fbe336d 100644
--- a/src/test/modules/test_misc/t/003_check_guc.pl
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -106,4 +106,45 @@ foreach my $param (@sample_intersect)
);
}
+# check if GUC parser accepts some unquoted values accepted by strtod()
+my $conffile = $node->data_dir . "/postgresql.conf";
+my $conf = slurp_file($conffile);
+## without units
+my $lines =
+ join("\n", map {"checkpoint_completion_target = $_"}
+ ('0.1', '1.', '.1', '1e-1', '.1e0', '1.e-01'));
+## with units
+$lines .= "\n" .
+ join("\n", map {"work_mem = ${_}MB"}
+ ('0.3', '1.', '.3', '+3e-1', '-.3e0', '1.e01'));
+$node->append_conf('postgresql.conf', $lines);
+
+$node->reload();
+## has the last value been read?
+ok ($node->poll_query_until('postgres',
+ "SELECT setting::int = 10240 FROM pg_settings WHERE name = 'work_mem'"),
+ 'all variations of real number parameter values passed');
+
+# check if parser correctly rejects some invalid patterns
+my $n = 0;
+foreach my $val ('.', '.e3')
+{
+ $n++ if (test_one_value($node, $val, $conf));
+}
+ok($n == 2, "parser succesfully rejected invalid values");
+
done_testing();
+
+sub test_one_value
+{
+ my ($node, $value, $base_conf) = @_;
+
+ open(my $fh, '>', $conffile) || die "failed to open $conffile";
+ print $fh $conf . "\nwork_mem=$value\n";
+ close($fh);
+
+ my $logstart = -s $node->logfile;
+ $node->reload();
+ return $node->wait_for_log('syntax error in file ".*postgresql.conf"',
+ $logstart);
+}
--
2.31.1
