Issue Details (XML | Word | Printable)

Key: CORE-5167
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Adriano dos Santos Fernandes
Reporter: Alex Bekhtin
Votes: 0
Watchers: 4
Operations

If you were logged in you would be able to see more operations.
Firebird Core

Allow implicit conversion between boolean and string

Created: 24/Mar/16 10:16 AM   Updated: 30/Apr/16 05:24 PM
Component/s: Engine
Affects Version/s: 3.0 RC2
Fix Version/s: 3.0.1, 4.0 Alpha 1

QA Status: Done successfully
Test Details: Checked on WI-T4.0.0.141; WI-V3.0.0.32490


 Description  « Hide
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


 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Adriano dos Santos Fernandes added a comment - 28/Mar/16 03:25 PM
Boolean is not implicit convertable to string, so it should not work with concatenation.

Alex Bekhtin added a comment - 30/Mar/16 12:43 PM
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

Adriano dos Santos Fernandes added a comment - 30/Mar/16 12:55 PM
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.

Sean Leyne added a comment - 30/Mar/16 03:10 PM - edited
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.

Adriano dos Santos Fernandes added a comment - 31/Mar/16 01:44 PM
Sean, and what Alex said and your link about SQL points we have a problem?

Sean Leyne added a comment - 31/Mar/16 02:02 PM
There is a string representation for Boolean datatypes ("TRUE", "FALSE" and "Unknown" or NULL), so "select '' || cast( TRUE as BOOLEAN ) ..." is valid.

Adriano dos Santos Fernandes added a comment - 31/Mar/16 02:17 PM
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.

Dmitry Yemanov added a comment - 31/Mar/16 02:30 PM
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.

Dmitry Yemanov added a comment - 31/Mar/16 02:45 PM
SQL:2011 draft is here: http://www.wiscorp.com/sql20nn.zip

Sean Leyne added a comment - 31/Mar/16 02:51 PM
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.

Dmitry Yemanov added a comment - 31/Mar/16 03:13 PM
I tend to agree with Sean.

Adriano dos Santos Fernandes added a comment - 31/Mar/16 03:27 PM
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?

Dmitry Yemanov added a comment - 31/Mar/16 03:44 PM
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?

Adriano dos Santos Fernandes added a comment - 31/Mar/16 04:33 PM
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;

Dmitry Yemanov added a comment - 31/Mar/16 04:44 PM
It should work by converting 'true' to boolean before comparing the values.

Adriano dos Santos Fernandes added a comment - 01/Apr/16 03:27 PM
But what about this?

not 'true'

'true' or 'false'

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

Agree?

Sean Leyne added a comment - 01/Apr/16 08:08 PM
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.

Dmitry Yemanov added a comment - 01/Apr/16 08:33 PM
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.

Sean Leyne added a comment - 01/Apr/16 08:42 PM
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) .

Dmitry Yemanov added a comment - 01/Apr/16 09:03 PM
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.

Sean Leyne added a comment - 01/Apr/16 09:28 PM
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'

Pavel Zotov added a comment - 06/Apr/16 10:15 AM
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

Pavel Zotov added a comment - 06/Apr/16 12:29 PM
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 ?

Adriano dos Santos Fernandes added a comment - 06/Apr/16 01:37 PM
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.

Pavel Zotov added a comment - 09/Apr/16 06:41 AM
<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

Pavel Zotov added a comment - 09/Apr/16 10:44 PM
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"

Pavel Zotov added a comment - 10/Apr/16 10:12 AM
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

Adriano dos Santos Fernandes added a comment - 11/Apr/16 12:54 AM
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.

Adriano dos Santos Fernandes added a comment - 11/Apr/16 12:57 AM
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...

Adriano dos Santos Fernandes added a comment - 11/Apr/16 12:59 AM
About BETWEEN, if we allow every crazy construct there, parser conflicts explodes.