[bug] Wrong bool value parameter
Do we allow such a bool parameter value? This seems puzzling to me.
postgres=# create table t1(c1 int) with(autovacuum_enabled ='tr');
CREATE TABLE
postgres=# create table t2(c1 int) with(autovacuum_enabled ='fa');
CREATE TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain | |
Access method: heap
Options: autovacuum_enabled=tr
postgres=# \d+ t2
Table "public.t2"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain | |
Access method: heap
Options: autovacuum_enabled=fa
I am try to fix in bug_boolrelopt.patch
Wenjing
Attachments:
bug_boolrelopt.patchapplication/octet-stream; name=bug_boolrelopt.patch; x-unix-mode=0644Download
diff --git a/src/backend/utils/adt/bool.c b/src/backend/utils/adt/bool.c
index 340607f..6e7368d 100644
--- a/src/backend/utils/adt/bool.c
+++ b/src/backend/utils/adt/bool.c
@@ -39,7 +39,7 @@ parse_bool_with_len(const char *value, size_t len, bool *result)
{
case 't':
case 'T':
- if (pg_strncasecmp(value, "true", len) == 0)
+ if (len == 1 || (len == 4 && pg_strncasecmp(value, "true", len) == 0))
{
if (result)
*result = true;
@@ -48,7 +48,7 @@ parse_bool_with_len(const char *value, size_t len, bool *result)
break;
case 'f':
case 'F':
- if (pg_strncasecmp(value, "false", len) == 0)
+ if (len == 1 || (len == 5 && pg_strncasecmp(value, "false", len) == 0))
{
if (result)
*result = false;
@@ -57,7 +57,7 @@ parse_bool_with_len(const char *value, size_t len, bool *result)
break;
case 'y':
case 'Y':
- if (pg_strncasecmp(value, "yes", len) == 0)
+ if (len == 1 || (len == 3 && pg_strncasecmp(value, "yes", len) == 0))
{
if (result)
*result = true;
On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012@gmail.com> wrote:
Do we allow such a bool parameter value? This seems puzzling to me.
postgres=# create table t1(c1 int) with(autovacuum_enabled ='tr');
CREATE TABLE
postgres=# create table t2(c1 int) with(autovacuum_enabled ='fa');
CREATE TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain |
|
Access method: heap
Options: autovacuum_enabled=tr[don't post to multiple mailing lists]
I'm not sure it is a bug. It certainly can be an improvement. Code as is
does not cause issues although I concur with you that it is at least a
strange syntax. It is like this at least since 2009 (commit ba748f7a11e).
I'm not sure parse_bool* is the right place to fix it because it could
break code. IMHO the problem is that parse_one_reloption() is using the
value provided by user; it should test those (abbreviation) conditions and
store "true" (for example) as bool value.
Regards,
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, 7 Apr 2020 at 20:58, Euler Taveira
<euler.taveira@2ndquadrant.com> wrote:
On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012@gmail.com> wrote:
Do we allow such a bool parameter value? This seems puzzling to me.
postgres=# create table t1(c1 int) with(autovacuum_enabled ='tr');
CREATE TABLE
postgres=# create table t2(c1 int) with(autovacuum_enabled ='fa');
CREATE TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain | |
Access method: heap
Options: autovacuum_enabled=tr[don't post to multiple mailing lists]
I'm not sure it is a bug. It certainly can be an improvement. Code as is does not cause issues although I concur with you that it is at least a strange syntax. It is like this at least since 2009 (commit ba748f7a11e). I'm not sure parse_bool* is the right place to fix it because it could break code. IMHO the problem is that parse_one_reloption() is using the value provided by user; it should test those (abbreviation) conditions and store "true" (for example) as bool value.
The document[1]https://www.postgresql.org/docs/devel/config-setting.html states:
Boolean: Values can be written as on, off, true, false, yes, no, 1, 0
(all case-insensitive) or any unambiguous prefix of one of these.
Given that PostgreSQL treats such values as boolean values it seems to
me that it's a normal behavior.
[1]: https://www.postgresql.org/docs/devel/config-setting.html
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
2020年4月7日 下午10:35，Masahiko Sawada <masahiko.sawada@2ndquadrant.com> 写道：
On Tue, 7 Apr 2020 at 20:58, Euler Taveira
<euler.taveira@2ndquadrant.com> wrote:On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012@gmail.com> wrote:
Do we allow such a bool parameter value? This seems puzzling to me.
postgres=# create table t1(c1 int) with(autovacuum_enabled ='tr');
CREATE TABLE
postgres=# create table t2(c1 int) with(autovacuum_enabled ='fa');
CREATE TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain | |
Access method: heap
Options: autovacuum_enabled=tr[don't post to multiple mailing lists]
I'm not sure it is a bug. It certainly can be an improvement. Code as is does not cause issues although I concur with you that it is at least a strange syntax. It is like this at least since 2009 (commit ba748f7a11e). I'm not sure parse_bool* is the right place to fix it because it could break code. IMHO the problem is that parse_one_reloption() is using the value provided by user; it should test those (abbreviation) conditions and store "true" (for example) as bool value.
The document[1] states:
Boolean: Values can be written as on, off, true, false, yes, no, 1, 0
(all case-insensitive) or any unambiguous prefix of one of these.Given that PostgreSQL treats such values as boolean values it seems to
me that it's a normal behavior.[1] https://www.postgresql.org/docs/devel/config-setting.html
Why do table parameters of a bool type have different rules than data types of a Boolean type?
postgres=# create table test_bool_type(a bool);
CREATE TABLE
postgres=# insert into test_bool_type values(true);
INSERT 0 1
postgres=# insert into test_bool_type values(false);
INSERT 0 1
postgres=# insert into test_bool_type values('false');
INSERT 0 1
postgres=# insert into test_bool_type values('t');
INSERT 0 1
postgres=# insert into test_bool_type values('f');
INSERT 0 1
postgres=# insert into test_bool_type values('tr');
ERROR: invalid input syntax for type boolean: "tr"
LINE 1: insert into test_bool_type values('tr');
^
postgres=# insert into test_bool_type values('fa');
ERROR: invalid input syntax for type boolean: "fa"
LINE 1: insert into test_bool_type values('fa');
^
postgres=# insert into test_bool_type values('fals');
ERROR: invalid input syntax for type boolean: "fals"
LINE 1: insert into test_bool_type values('fals');
Show quoted text
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, 8 Apr 2020 at 16:00, wenjing <wjzeng2012@gmail.com> wrote:
2020年4月7日 下午10:35，Masahiko Sawada <masahiko.sawada@2ndquadrant.com> 写道：
On Tue, 7 Apr 2020 at 20:58, Euler Taveira
<euler.taveira@2ndquadrant.com> wrote:On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012@gmail.com> wrote:
Do we allow such a bool parameter value? This seems puzzling to me.
postgres=# create table t1(c1 int) with(autovacuum_enabled ='tr');
CREATE TABLE
postgres=# create table t2(c1 int) with(autovacuum_enabled ='fa');
CREATE TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain | |
Access method: heap
Options: autovacuum_enabled=tr[don't post to multiple mailing lists]
I'm not sure it is a bug. It certainly can be an improvement. Code as is does not cause issues although I concur with you that it is at least a strange syntax. It is like this at least since 2009 (commit ba748f7a11e). I'm not sure parse_bool* is the right place to fix it because it could break code. IMHO the problem is that parse_one_reloption() is using the value provided by user; it should test those (abbreviation) conditions and store "true" (for example) as bool value.
The document[1] states:
Boolean: Values can be written as on, off, true, false, yes, no, 1, 0
(all case-insensitive) or any unambiguous prefix of one of these.Given that PostgreSQL treats such values as boolean values it seems to
me that it's a normal behavior.[1] https://www.postgresql.org/docs/devel/config-setting.html
Why do table parameters of a bool type have different rules than data types of a Boolean type?
postgres=# create table test_bool_type(a bool);
CREATE TABLE
postgres=# insert into test_bool_type values(true);
INSERT 0 1
postgres=# insert into test_bool_type values(false);
INSERT 0 1
postgres=# insert into test_bool_type values('false');
INSERT 0 1
postgres=# insert into test_bool_type values('t');
INSERT 0 1
postgres=# insert into test_bool_type values('f');
INSERT 0 1postgres=# insert into test_bool_type values('tr');
ERROR: invalid input syntax for type boolean: "tr"
LINE 1: insert into test_bool_type values('tr');
^
postgres=# insert into test_bool_type values('fa');
ERROR: invalid input syntax for type boolean: "fa"
LINE 1: insert into test_bool_type values('fa');
^
postgres=# insert into test_bool_type values('fals');
ERROR: invalid input syntax for type boolean: "fals"
LINE 1: insert into test_bool_type values('fals');
Hmm that seems strange. In my environment, both 'tr' and 'fa' are
accepted at least with the current HEAD
postgres(1:52514)=# insert into test_bool_type values('tr');
INSERT 0 1
postgres(1:52514)=# insert into test_bool_type values('fa');
INSERT 0 1
postgres(1:52514)=# insert into test_bool_type values('fals');
INSERT 0 1
IIUC both bool of SQL data type and bool of GUC parameter type are
using the same function parse_bool_with_len() to parse the input
value. The behavior can vary depending on the environment?
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Masahiko Sawada <masahiko.sawada@2ndquadrant.com> writes:
On Wed, 8 Apr 2020 at 16:00, wenjing <wjzeng2012@gmail.com> wrote:
Why do table parameters of a bool type have different rules than data types of a Boolean type?
postgres=# insert into test_bool_type values('fals');
ERROR: invalid input syntax for type boolean: "fals"
LINE 1: insert into test_bool_type values('fals');
Hmm that seems strange. In my environment, both 'tr' and 'fa' are
accepted at least with the current HEAD
Yeah, it works for me too:
regression=# select 'fa'::bool;
bool
------
f
(1 row)
regression=# select 'fals'::bool;
bool
------
f
(1 row)
IIUC both bool of SQL data type and bool of GUC parameter type are
using the same function parse_bool_with_len() to parse the input
value. The behavior can vary depending on the environment?
parse_bool_with_len is not locale-sensitive for ASCII input.
Conceivably its case folding could vary for non-ASCII, but that's
not relevant here.
I am suspicious that the OP is not using community Postgres.
This seems like the kind of thing that EDB might've hacked
for better Oracle compatibility, for example.
regards, tom lane
2020年4月8日 21:45，Tom Lane <tgl@sss.pgh.pa.us> 写道：
Masahiko Sawada <masahiko.sawada@2ndquadrant.com> writes:
On Wed, 8 Apr 2020 at 16:00, wenjing <wjzeng2012@gmail.com> wrote:
Why do table parameters of a bool type have different rules than data types of a Boolean type?
postgres=# insert into test_bool_type values('fals');
ERROR: invalid input syntax for type boolean: "fals"
LINE 1: insert into test_bool_type values('fals');Hmm that seems strange. In my environment, both 'tr' and 'fa' are
accepted at least with the current HEADYeah, it works for me too:
regression=# select 'fa'::bool;
bool
------
f
(1 row)regression=# select 'fals'::bool;
bool
------
f
(1 row)IIUC both bool of SQL data type and bool of GUC parameter type are
using the same function parse_bool_with_len() to parse the input
value. The behavior can vary depending on the environment?
parse_bool_with_len is not locale-sensitive for ASCII input.
Conceivably its case folding could vary for non-ASCII, but that's
not relevant here.I am suspicious that the OP is not using community Postgres.
This seems like the kind of thing that EDB might've hacked
for better Oracle compatibility, for example.
Sorry, you're right. I used the modified code and got the wrong result.
Show quoted text
regards, tom lane
2020年4月7日 22:35，Masahiko Sawada <masahiko.sawada@2ndquadrant.com> 写道：
On Tue, 7 Apr 2020 at 20:58, Euler Taveira
<euler.taveira@2ndquadrant.com> wrote:On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012@gmail.com> wrote:
Do we allow such a bool parameter value? This seems puzzling to me.
postgres=# create table t1(c1 int) with(autovacuum_enabled ='tr');
CREATE TABLE
postgres=# create table t2(c1 int) with(autovacuum_enabled ='fa');
CREATE TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
c1 | integer | | | | plain | |
Access method: heap
Options: autovacuum_enabled=tr[don't post to multiple mailing lists]
I'm not sure it is a bug. It certainly can be an improvement. Code as is does not cause issues although I concur with you that it is at least a strange syntax. It is like this at least since 2009 (commit ba748f7a11e). I'm not sure parse_bool* is the right place to fix it because it could break code. IMHO the problem is that parse_one_reloption() is using the value provided by user; it should test those (abbreviation) conditions and store "true" (for example) as bool value.
It seems difficult to store a new bool value in parse_one_reloption. This is a string stored with ”autovacuum_enabled =“.
any other ideas?
Show quoted text
The document[1] states:
Boolean: Values can be written as on, off, true, false, yes, no, 1, 0
(all case-insensitive) or any unambiguous prefix of one of these.Given that PostgreSQL treats such values as boolean values it seems to
me that it's a normal behavior.[1] https://www.postgresql.org/docs/devel/config-setting.html
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
wenjing <wjzeng2012@gmail.com> writes:
2020年4月7日 22:35，Masahiko Sawada <masahiko.sawada@2ndquadrant.com> 写道：
I'm not sure it is a bug. It certainly can be an improvement. Code as is does not cause issues although I concur with you that it is at least a strange syntax. It is like this at least since 2009 (commit ba748f7a11e). I'm not sure parse_bool* is the right place to fix it because it could break code. IMHO the problem is that parse_one_reloption() is using the value provided by user; it should test those (abbreviation) conditions and store "true" (for example) as bool value.
It seems difficult to store a new bool value in parse_one_reloption. This is a string stored with ”autovacuum_enabled =“.
any other ideas?
I don't think we should touch this. If the user chose to write the value
in a specific way, they might've had a reason for that. There's little
reason for us to override it, certainly not enough to justify introducing
a lot of new mechanism just to do that.
regards, tom lane