Re: [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

On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012(at)gmail(dot)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(dot)taveira(at)2ndquadrant(dot)com> wrote:
>
> On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012(at)gmail(dot)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] /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(dot)sawada(at)2ndquadrant(dot)com> 写道：
>
> On Tue, 7 Apr 2020 at 20:58, Euler Taveira
> <euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>>
>> On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012(at)gmail(dot)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] /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');

>
> 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(at)gmail(dot)com> wrote:
>
>
>
> 2020年4月7日 下午10:35，Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> 写道：
>
> On Tue, 7 Apr 2020 at 20:58, Euler Taveira
> <euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>
>
> On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012(at)gmail(dot)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] /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');
>

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(dot)sawada(at)2ndquadrant(dot)com> writes:
> On Wed, 8 Apr 2020 at 16:00, wenjing <wjzeng2012(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us> 写道：
>
> Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> writes:
>> On Wed, 8 Apr 2020 at 16:00, wenjing <wjzeng2012(at)gmail(dot)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.
Sorry, you're right. I used the modified code and got the wrong result.

>
> regards, tom lane

> 2020年4月7日 22:35，Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> 写道：
>
> On Tue, 7 Apr 2020 at 20:58, Euler Taveira
> <euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>>
>> On Tue, 7 Apr 2020 at 06:30, 曾文旌 <wjzeng2012(at)gmail(dot)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?

>>
>
> 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] /docs/devel/config-setting.html
>
> Regards,
>
> --
> Masahiko Sawada http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

wenjing <wjzeng2012(at)gmail(dot)com> writes:
> 2020年4月7日 22:35，Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)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