From e4ff1cb2917d9f6fb2a9482e46b376580e25921b Mon Sep 17 00:00:00 2001
From: Greg Nancarrow <gregn4422@gmail.com>
Date: Mon, 24 Aug 2020 17:43:22 +1000
Subject: [PATCH v1] Fix GUC parse_int() to allow fractional input only when a 
 unit is accepted.

The following commit made some changes to how integer GUC option values are
parsed and what values are allowed, allowing fractional input and rounding
to the nearest multiple of the next smaller unit:

https://github.com/postgres/postgres/commit/1a83a80a2fe5b559f85ed4830acb92d5124b7a9a

However, it also allowed fractional input for cases it probably shouldn't
have (i.e. when the setting does not accept a unit), such as:

log_file_mode = 384.234
max_connections = 1.0067e2
port = 5432.123

Also, parse_int() is used for parsing other options, such as the integer
storage parameters for CREATE TABLE and CREATE INDEX. For example, the
following are currently allowed but probably shouldn't be:

CREATE TABLE ... WITH (fillfactor = 23.45);
CREATE TABLE ... WITH (parallel_workers = 5.4);

A couple of related test items are affected by this fix.
---
 src/backend/utils/misc/guc.c                            | 11 +++++++----
 src/test/modules/dummy_index_am/expected/reloptions.out | 10 +++++-----
 src/test/modules/dummy_index_am/sql/reloptions.sql      |  2 +-
 src/test/regress/expected/reloptions.out                |  5 ++---
 src/test/regress/sql/reloptions.sql                     |  2 +-
 5 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index de87ad6..ac47abf 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6514,11 +6514,14 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 	 */
 	errno = 0;
 	val = strtol(value, &endptr, 0);
-	if (*endptr == '.' || *endptr == 'e' || *endptr == 'E' ||
-		errno == ERANGE)
+	if (flags & GUC_UNIT)
 	{
-		errno = 0;
-		val = strtod(value, &endptr);
+			if (*endptr == '.' || *endptr == 'e' || *endptr == 'E' ||
+				errno == ERANGE)
+			{
+				errno = 0;
+				val = strtod(value, &endptr);
+			}
 	}
 
 	if (endptr == value || errno == ERANGE)
diff --git a/src/test/modules/dummy_index_am/expected/reloptions.out b/src/test/modules/dummy_index_am/expected/reloptions.out
index c873a80..4592c7b 100644
--- a/src/test/modules/dummy_index_am/expected/reloptions.out
+++ b/src/test/modules/dummy_index_am/expected/reloptions.out
@@ -73,16 +73,16 @@ SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
 
 -- Cross-type checks for reloption values
 -- Integer
-ALTER INDEX dummy_test_idx SET (option_int = 3.3); -- ok
+ALTER INDEX dummy_test_idx SET (option_int = 3.3); -- error
+ERROR:  invalid value for integer option "option_int": 3.3
 ALTER INDEX dummy_test_idx SET (option_int = true); -- error
 ERROR:  invalid value for integer option "option_int": true
 ALTER INDEX dummy_test_idx SET (option_int = 'val3'); -- error
 ERROR:  invalid value for integer option "option_int": val3
 SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
-     unnest     
-----------------
- option_int=3.3
-(1 row)
+ unnest 
+--------
+(0 rows)
 
 ALTER INDEX dummy_test_idx RESET (option_int);
 -- Boolean
diff --git a/src/test/modules/dummy_index_am/sql/reloptions.sql b/src/test/modules/dummy_index_am/sql/reloptions.sql
index 6749d76..cac0732 100644
--- a/src/test/modules/dummy_index_am/sql/reloptions.sql
+++ b/src/test/modules/dummy_index_am/sql/reloptions.sql
@@ -48,7 +48,7 @@ SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
 
 -- Cross-type checks for reloption values
 -- Integer
-ALTER INDEX dummy_test_idx SET (option_int = 3.3); -- ok
+ALTER INDEX dummy_test_idx SET (option_int = 3.3); -- error
 ALTER INDEX dummy_test_idx SET (option_int = true); -- error
 ALTER INDEX dummy_test_idx SET (option_int = 'val3'); -- error
 SELECT unnest(reloptions) FROM pg_class WHERE relname = 'dummy_test_idx';
diff --git a/src/test/regress/expected/reloptions.out b/src/test/regress/expected/reloptions.out
index 44c1304..4e1338f 100644
--- a/src/test/regress/expected/reloptions.out
+++ b/src/test/regress/expected/reloptions.out
@@ -26,9 +26,8 @@ ERROR:  unrecognized parameter "not_existing_option"
 CREATE TABLE reloptions_test2(i INT) WITH (not_existing_namespace.fillfactor=2);
 ERROR:  unrecognized parameter namespace "not_existing_namespace"
 -- Fail while setting improper values
-CREATE TABLE reloptions_test2(i INT) WITH (fillfactor=-30.1);
-ERROR:  value -30.1 out of bounds for option "fillfactor"
-DETAIL:  Valid values are between "10" and "100".
+CREATE TABLE reloptions_test2(i INT) WITH (fillfactor=30.5);
+ERROR:  invalid value for integer option "fillfactor": 30.5
 CREATE TABLE reloptions_test2(i INT) WITH (fillfactor='string');
 ERROR:  invalid value for integer option "fillfactor": string
 CREATE TABLE reloptions_test2(i INT) WITH (fillfactor=true);
diff --git a/src/test/regress/sql/reloptions.sql b/src/test/regress/sql/reloptions.sql
index cac5b0b..09e7daa 100644
--- a/src/test/regress/sql/reloptions.sql
+++ b/src/test/regress/sql/reloptions.sql
@@ -15,7 +15,7 @@ CREATE TABLE reloptions_test2(i INT) WITH (not_existing_option=2);
 CREATE TABLE reloptions_test2(i INT) WITH (not_existing_namespace.fillfactor=2);
 
 -- Fail while setting improper values
-CREATE TABLE reloptions_test2(i INT) WITH (fillfactor=-30.1);
+CREATE TABLE reloptions_test2(i INT) WITH (fillfactor=30.5);
 CREATE TABLE reloptions_test2(i INT) WITH (fillfactor='string');
 CREATE TABLE reloptions_test2(i INT) WITH (fillfactor=true);
 CREATE TABLE reloptions_test2(i INT) WITH (autovacuum_enabled=12);
-- 
1.8.3.1

