[bug] Wrong bool value parameter

Started by wenjingover 6 years ago9 messageshackersbugs
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliestests failedCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t42186
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 20, 2026 at 12:19 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t42186_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t42186_1 && git checkout t42186_1

Patchset v1 (message #1) is on t42186_1

Jump to latest
#1wenjing
wjzeng2012@gmail.com
hackersbugs

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:

t42186_1
bug_boolrelopt.patchapplication/octet-stream; name=bug_boolrelopt.patch; x-unix-mode=0644Download+3-3
#2Euler Taveira
euler.taveira@2ndquadrant.com
In reply to: wenjing (#1)
hackersbugs
Re: [bug] Wrong bool value parameter

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

#3Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Euler Taveira (#2)
hackersbugs
Re: [bug] Wrong bool value parameter

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

#4wenjing
wjzeng2012@gmail.com
In reply to: Masahiko Sawada (#3)
hackersbugs
Re: [bug] Wrong bool value parameter

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

#5Masahiko Sawada
sawada.mshk@gmail.com
In reply to: wenjing (#4)
hackersbugs
Re: [bug] Wrong bool value parameter

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 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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Masahiko Sawada (#5)
hackersbugs
Re: [bug] Wrong bool value parameter

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

#7wenjing
wjzeng2012@gmail.com
In reply to: Tom Lane (#6)
hackersbugs
Re: [bug] Wrong bool value parameter

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 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.

Show quoted text

regards, tom lane

#8wenjing
wjzeng2012@gmail.com
In reply to: Masahiko Sawada (#3)
hackersbugs
Re: [bug] Wrong bool value parameter

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

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: wenjing (#8)
hackersbugs
Re: [bug] Wrong bool value parameter

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