Real config values for bytes needs quotes?
Hello.
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.
Is the following a correct English sentense?
Do you folks think this makes sense?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
guc-accept-real-values-follows-by-unit.patchtext/x-patch; charset=us-asciiDownload
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 41d62a9f23..6727f1952f 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}*{EXPONENT}?|{EXPONENT}){UNIT_LETTER}*
LETTER [A-Za-z_\200-\377]
LETTER_OR_DIGIT [A-Za-z_0-9\200-\377]
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. :)
At Thu, 2 Mar 2023 11:54:10 +0100, Peter Eisentraut <peter.eisentraut@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
Attachments:
v1-0001-Widen-unquoted-value-acceptance-in-configuration-.patchtext/x-patch; charset=us-asciiDownload
From 9a413b791364152d44838d329f0efe3cc7db4d22 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@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