Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow implicit conversion between boolean and string [CORE5167] #5450

Closed
firebird-issue-importer opened this issue Mar 24, 2016 · 42 comments
Closed

Comments

@firebird-issue-importer

Submitted by: Alex Bekhtin (afgm)

Edited subject to say about the more general case - Adriano.

select '' || cast( 1 as SMALLINT ) from rdb$database;
select '' || cast( 1 as INTEGER ) from rdb$database;
select '' || cast( 1 as BIGINT ) from rdb$database;
select '' || cast( TRUE as BOOLEAN ) from rdb$database; -- Overflow occurred during data type conversion.
-- conversion error from string "BOOLEAN".
select '' || cast( 1.1 as FLOAT ) from rdb$database;
select '' || cast( 1.1 as DOUBLE PRECISION ) from rdb$database;
select '' || cast( 1.1 as NUMERIC(3,3) ) from rdb$database;
select '' || cast( 1.1 as DECIMAL(3,3) ) from rdb$database;
select '' || cast( '2015-01-01' as DATE ) from rdb$database;
select '' || cast( '10:51:59' as TIME ) from rdb$database;
select '' || cast( '2015-01-01 10:51:59' as TIMESTAMP ) from rdb$database;
select '' || cast( 'char text' as CHAR(20) ) from rdb$database;
select '' || cast( 'varchar text' as VARCHAR(20) ) from rdb$database;
select '' || cast( 'blob text' as BLOB ) from rdb$database;

select cast( TRUE as varchar(20) ) from rdb$database; -- works fine

Commits: 1635a93 251ec1c d7eb6fe

====== Test Details ======

Checked on WI-T4.0.0.141; WI-V3.0.0.32490

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 28, 2016

Commented by: @asfernandes

Boolean is not implicit convertable to string, so it should not work with concatenation.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 28, 2016

Modified by: @asfernandes

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Won't Fix [ 2 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 29, 2016

Modified by: @pcisar

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 30, 2016

Commented by: Alex Bekhtin (afgm)

But why?
|| - 2 steps operator:
1. pseudo-imlicit conversion (explicit de facto)
2. string concatenation

and

SQL-2003:
<concatenation> ::= <character value expression> <concatenation operator> <character factor>
<concatenation operator> ::= <vertical bar> <vertical bar>

<character factor> ::= <character primary> [ <collate clause> ]
<character primary> ::= <value expression primary> | <string value function>
<value expression primary> ::=
<parenthesized value expression>
| <nonparenthesized value expression primary>
<nonparenthesized value expression primary> ::=
<unsigned value specification>
| ...

<unsigned value specification> ::= <unsigned literal> | <general value specification>
<unsigned literal> ::= <unsigned numeric literal> | <general literal>
<general literal> ::=
<character string literal>
| <national character string literal>
| <Unicode character string literal>
| <binary string literal>
| <datetime literal>
| <interval literal>
| <boolean literal>
<boolean literal> ::= TRUE | FALSE | UNKNOWN

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 30, 2016

Commented by: @asfernandes

So, you're saying if it's implicitly convertible, it would work, but I said it's not implicit convertible per the standard.

I didn't found where this is said now. But feel free to point a place telling the contrary.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 30, 2016

Commented by: Sean Leyne (seanleyne)

Boolean string literals were defined in the SQL:1999 standard (https://en.wikipedia.org/wiki/Boolean_data_type) -- the details align with details posted above Alex

The Oracle semantics (http://docs.oracle.com/cd/B28359_01/appdev.111/b28370/literal.htm) -- note they use "NULL" not "UNKNOWN"

The PostgreSQL semantics (http://www.postgresql.org/docs/8.1/static/datatype-boolean.html) -- they also refer to "NULL" for "UNKNOWN" but the link doesn't provide full example to confirm

IMO, this case should be re-opened, this is a real issue.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Modified by: Sean Leyne (seanleyne)

status: Closed [ 6 ] => Reopened [ 4 ]

resolution: Won't Fix [ 2 ] =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @asfernandes

Sean, and what Alex said and your link about SQL points we have a problem?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: Sean Leyne (seanleyne)

There is a string representation for Boolean datatypes ("TRUE", "FALSE" and "Unknown" or NULL), so "select '' || cast( TRUE as BOOLEAN ) ..." is valid.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @asfernandes

Yes, and that is the explicit conversion. This ticket is about implicit conversion.

I'm trying to find the SQL 2011 foundation PDF but no luck. But I do remember it disallowed implicit conversion of booleans to strings.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @dyemanov

It's not as simple. SQL spec explicitly forbids many implicit conversions that we support since the very beginning. It has a special chapter "9.5 Result of data type combinations" which is applied for concatenation (among other operations). This chapter declares that, for example:

If any of the data types in DTS is character string, then:
i) All data types in DTS shall be character string, and all of them shall have the same character
repertoire. The character set of the result is the character set of the data type in DTS that has
the character encoding form with the highest precedence.

It means that 'asd' || 123 must be prohibited. But it's perfectly valid in Firebird. The same for other datatypes.

So, if we follow the SQL spec for booleans, then the current implementation is correct but it works opposite to all other datatypes in Firebird. If we follow our historical behaviour, then implicit conversions should be allowed but it would violate the standard.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @dyemanov

SQL:2011 draft is here: http://www.wiscorp.com/sql20nn.zip

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: Sean Leyne (seanleyne)

As I see it, given that our current implementation is non-conforming, we have the following choices.

1- keep the current implementation

2- provide implicit conversion for Boolean to maintain consistency with the non-conforming treatment of other data types

3- adopt conforming treatment for all "data type combinations"

IMO, #⁠2 seems to be the only reasonable path.

#⁠3 would create significant compatibility issues with 99.99% of existing applications, this option is not reasonable.

#⁠1 would create exceptional/inconsistent treatment. It is better to be consistent, even if non-conforming, then to be inconsistent -- developers want/can 'handle' consistent/predicable handling, they can't abide inconsistency.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @dyemanov

I tend to agree with Sean.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @asfernandes

The behaviour of implicit conversion with boolean was not logical for me in relation to what we had, but seems it what I found when looking for BOOLEAN things in the standard.

It's surely inconsistent with Firebird handling of other types.

It is, however, documented: Booleans are not implicitly convertible to any other datatype. But it's convertible to/from strings with CAST.

And that may be relaxed without problems.

So should we change it only for v4?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @dyemanov

I believe this should be relaxed for v3 too. But I'm afraid this is too late for v3.0.0 (not only code must be changes but also docs), so maybe in v3.0.1?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Modified by: @asfernandes

issuetype: Bug [ 1 ] => Improvement [ 4 ]

Fix Version: 4.0 Alpha 1 [ 10731 ]

assignee: Adriano dos Santos Fernandes [ asfernandes ]

description: select '' || cast( 1 as SMALLINT ) from rdb$database;
select '' || cast( 1 as INTEGER ) from rdb$database;
select '' || cast( 1 as BIGINT ) from rdb$database;
select '' || cast( TRUE as BOOLEAN ) from rdb$database; -- Overflow occurred during data type conversion.
-- conversion error from string "BOOLEAN".
select '' || cast( 1.1 as FLOAT ) from rdb$database;
select '' || cast( 1.1 as DOUBLE PRECISION ) from rdb$database;
select '' || cast( 1.1 as NUMERIC(3,3) ) from rdb$database;
select '' || cast( 1.1 as DECIMAL(3,3) ) from rdb$database;
select '' || cast( '2015-01-01' as DATE ) from rdb$database;
select '' || cast( '10:51:59' as TIME ) from rdb$database;
select '' || cast( '2015-01-01 10:51:59' as TIMESTAMP ) from rdb$database;
select '' || cast( 'char text' as CHAR(20) ) from rdb$database;
select '' || cast( 'varchar text' as VARCHAR(20) ) from rdb$database;
select '' || cast( 'blob text' as BLOB ) from rdb$database;

select cast( TRUE as varchar(20) ) from rdb$database; -- works fine

=>

Edited subject to say about the more general case - Adriano.

select '' || cast( 1 as SMALLINT ) from rdb$database;
select '' || cast( 1 as INTEGER ) from rdb$database;
select '' || cast( 1 as BIGINT ) from rdb$database;
select '' || cast( TRUE as BOOLEAN ) from rdb$database; -- Overflow occurred during data type conversion.
-- conversion error from string "BOOLEAN".
select '' || cast( 1.1 as FLOAT ) from rdb$database;
select '' || cast( 1.1 as DOUBLE PRECISION ) from rdb$database;
select '' || cast( 1.1 as NUMERIC(3,3) ) from rdb$database;
select '' || cast( 1.1 as DECIMAL(3,3) ) from rdb$database;
select '' || cast( '2015-01-01' as DATE ) from rdb$database;
select '' || cast( '10:51:59' as TIME ) from rdb$database;
select '' || cast( '2015-01-01 10:51:59' as TIMESTAMP ) from rdb$database;
select '' || cast( 'char text' as CHAR(20) ) from rdb$database;
select '' || cast( 'varchar text' as VARCHAR(20) ) from rdb$database;
select '' || cast( 'blob text' as BLOB ) from rdb$database;

select cast( TRUE as varchar(20) ) from rdb$database; -- works fine

summary: Automatic boolean values convertation with string concatenation => Allow implicit conversion of boolean values to string

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @asfernandes

Should this continue to give an error?

SQL> select false > 'true' from rdb$database;
Statement failed, SQLSTATE = 22000
Dynamic SQL Error
-SQL error code = -104
-Invalid usage of boolean expression

Or should not like this?

select 1 > '1' from rdb$database;

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Commented by: @dyemanov

It should work by converting 'true' to boolean before comparing the values.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 1, 2016

Commented by: @asfernandes

But what about this?

not 'true'

'true' or 'false'

IMO, should give an error like '1' + '2' does.

Agree?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 1, 2016

Commented by: Sean Leyne (seanleyne)

Dmitry,

What does the SQL standard say about case sensitivity of Boolean constants?

PostgreSQL (http://www.postgresql.org/docs/8.1/static/datatype-boolean.html), to my reading, describes the only valid non-string/non-quoted representations for Booleans as TRUE and FALSE.

So, while 'true' and 'false' are a valid Boolean string values, "false" or "true" are not valid non-string representations.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 1, 2016

Commented by: @dyemanov

Adriano, given our legacy dialect-related issues, I woudn't take +-*/ as good examples, they have hard historical heritage. But speaking practically, I think it would be OK for your cases to throw an error.

Sean, "something" is an identifier, so double quotes are out of question.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 1, 2016

Commented by: Sean Leyne (seanleyne)

Dmitry,

I was trying to use " to denote a non-quoted literal (not as an identifier), as in:

select FALSE \> 'true' from rdb$database;

is valid (since FALSE and 'true' are valid Boolean values) whereas

select false \> 'true' from rdb$database

would be invalid (since false is not a valid Boolean value) .

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 2, 2016

Commented by: @dyemanov

In your terms, both "FALSE" and "false" mean the same in our grammar (and this is OK from the standard POV), so I suppose this implies that both 'FALSE' and 'false' should be impicitly converted to boolean without errors. The standard is not absolutely clear about that, but its rules for CAST declare that if the string to be converted from matches <literal> element, it should be converted. <literal> for booleans define one of: TRUE, FALSE, UNKNOWN. But following the generic grammar, these tokens are case-insensitive.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 2, 2016

Commented by: Sean Leyne (seanleyne)

Dmitry,

Thanks for the clarification.

Based on same, it seems that of Adriano's examples, these are valid:

select false \> 'true' from rdb$database; 
select 1 \> '1' from rdb$database; 
'true' or 'false' 

Personally, I am not sure about:
not 'true'

A strict reading of the PostgreSQL pages suggest that this is not valid, the correct syntax being IS NOT 'true'

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 4, 2016

Modified by: @asfernandes

Component: Engine [ 10000 ]

summary: Allow implicit conversion of boolean values to string => Allow implicit conversion between boolean and string

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 4, 2016

Modified by: @asfernandes

status: Reopened [ 4 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 6, 2016

Commented by: @pavel-zotov

Can anyone clarify what's wrong here:

select 'false' <> not false from rdb$database; -- output: <true>; expected
select 'true' = not false from rdb$database; -- output: <true>; expected
select 'true' is not false from rdb$database; -- output: <true>; expected

But:

select true = not 'false' from rdb$database;
Statement failed, SQLSTATE = 22000
Dynamic SQL Error
-SQL error code = -104
-Invalid usage of boolean expression

select true is not 'false' from rdb$database;
Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 1, column 20
-'false'

PS. Checked on: WI-T4.0.0.119

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 6, 2016

Commented by: @pavel-zotov

One more sample, without 'not':

SQL> select true = 'true' from rdb$database; -- output: <true>; expected

SQL> select true is 'true' from rdb$database;
Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 1, column 16
-'true'

'IS' does not like string literal in the right part of expression ?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 6, 2016

Commented by: @asfernandes

Pavel, true is 'true' is invalid. The operator is IS [NOT] {TRUE | FALSE} not IS [NOT] <value>

not 'false' as I asked, together with AND/OR, does not allow non boolean argument.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 9, 2016

Commented by: @pavel-zotov

<boolean literal> ::= TRUE | FALSE | UNKNOWN

What about literal UNKNOWN usage ? Should it be always treated as NULL ?

SQL> select true is null from rdb$database; -- <false> -- OK, expected

SQL> select 'true' is null from rdb$database; -- <false> -- OK, expected

SQL> select true is unknown from rdb$database; -- <false> -- OK, expected

SQL> select 'true' is unknown from rdb$database;
Statement failed, SQLSTATE = 22000
Dynamic SQL Error
-SQL error code = -104
-Invalid usage of boolean expression --------------------------------------- ? WHY ?

SQL> select 'true' is true from rdb$database; -- <true>

SQL> select 'true' is false from rdb$database; -- <false>

PS. WI-T4.0.0.127

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 10, 2016

Commented by: @pavel-zotov

Am I right in guess that 'UNKNOW' (enclosed in single quotes) can be used on when it's compared with UNKNOWN literal and _not_ with TRUE | FALSE ?

select unknown in ('unknown', 'false', 'true') from rdb$database; -- stdOut: <null>

select true in ('unknown', 'false', 'true') from rdb$database; -- stdErr: SQLSTATE = 22018 // conversion error from string "unknown"

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 10, 2016

Commented by: @pavel-zotov

It seems that BETWEEN also has some troubles with boolean expressions:

SQL> select true >= not true and true <= not false from rdb$database; -- <true>; OK

SQL> select true between (not true) and (not false) from rdb$database; -- <true>; OK

SQL> select true between not true and not false from rdb$database;
Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 1, column 21
-not

SQL> select true between not true and (not false) from rdb$database;
Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 1, column 21
-not

SQL> select true between (not true) and not false from rdb$database;
Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 1, column 36
-not

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 11, 2016

Commented by: @asfernandes

About:
select 'true' is unknown from rdb$database; -- gives error
and
select 'true' is true from rdb$database; -- <true>

My question is not why about the first, but how both should be, as they should be equivalent.

Following the logic of AND/OR giving an error as they are exclusive boolean operators, I tend to think that the correct should be raise error for both.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 11, 2016

Commented by: @asfernandes

IMO, cast 'UNKNOWN' to boolean should raise an error like cast 'NULL' to date/number does. May be not what the standard say, but give the fact about all differences between Firebird and the standard in relation to everything about this matter discussed here...

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 11, 2016

Commented by: @asfernandes

About BETWEEN, if we allow every crazy construct there, parser conflicts explodes.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 11, 2016

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: No test => Deferred

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 16, 2016

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: Deferred =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 22, 2016

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: Done successfully

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 22, 2016

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 23, 2016

Modified by: @pavel-zotov

status: Closed [ 6 ] => Closed [ 6 ]

Test Details: Checked on WI-T4.0.0.141; WI-V3.0.0.32490

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 30, 2016

Modified by: @dyemanov

Fix Version: 3.0.1 [ 10730 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants