SQLite and Boolean value of -1
lazarus / 16-Databases

----------------------------------------------------------------------
rvk · 2015-06-12 12:34

Has anybody else found a problem with Boolean fields in SQLite in
combination with Lazarus??

Following example:
Code: [Select]procedure TForm1.FormCreate(Sender: TObject);
var
  SQL: string;
begin
  Deletefile('test.db'); // remove if you want to keep de db
  if not FileExists('test.db') then
  begin
    SQL := 'CREATE TABLE testtable(id int PRIMARY KEY ASC, name
string, bol boolean);';
    SQLite3Connection1.ExecuteDirect(SQL);
    SQL := 'INSERT INTO testtable VALUES (1, ''first'',0);';
    SQLite3Connection1.ExecuteDirect(SQL);
    SQL := 'INSERT INTO testtable VALUES (2, ''second'',0);';
    SQLite3Connection1.ExecuteDirect(SQL);
    SQL := 'INSERT INTO testtable VALUES (3, ''third'',1);';
    SQLite3Connection1.ExecuteDirect(SQL);
  end;
  SQLite3Connection1.Connected := True;
  SQLQuery1.Upda]"]>Blockedde := upWhereAll; // <---- This won't work
because asBoolean = -1 (see later)

  SQLQuery1.Upda]"]>Blockedde := upWhereKeyOnly;

  SQLQuery1.Open;
  SQLQuery1.Edit;
  SQLQuery1.FieldByName('name').AsString := 'changed';
  SQLQuery1.FieldByName('bol').AsBoolean := true;
  SQLQuery1.Post;
  SQLQuery1.ApplyUpdates;
  SQLite3Connection1.Transaction.CommitRetaining;

  // now record 1 has -1 in the bol field according to sqliteman.exe

end;

This example should create a table with 3 records. I set
Upda]"]>Blockedde to upWhereKeyOnly and have a Primary index.

Now I change the first record field bol to asBoolean := true;
The field bol should contain 1 in the database but when I look with
sqliteman.exe it has -1.
(it should be 0 or 1 according to
https://www.sqlite.org/datatype3.html and never -1)


With the same problem, when having 0 and 1 in the database (for bol
values) when using Upda]"]>Blockedde upWhereAll, it won't work because
Lazarus checks for OLD_bol = -1 while the database is filled with 1.

Has anybody ever seen this?
If not, I'll open a bug-report.


Edit:
I think the problem lies in the fact that this results in -1:
Code: [Select]var
  b1: boolean;
  b2: wordbool;
begin
  b1 := true;
  b2 := true;
  Showmessage('Boolean true = ' + inttostr(ord(b1)) + ' Wordbool true
= ' + inttostr(ord(b2)));
Result:
Code: [Select]Boolean true = 1 Wordbool true = -1
And:
Code: [Select]procedure TBooleanField.SetAsBoolean(AValue: Boolean);
var b : wordbool;
begin
  b := AValue;
  SetData(@b);
end;

Is this also true in other DB engines where Boolean is translated to a
integer-field internally?

Should this be reported as a bug in SQLite (sqldb.pp for not
overriding SetAsBoolean) or for the global fields.inc for not doing
Boolean = true = 1 ??

----------------------------------------------------------------------
taazz · 2015-06-12 13:19

make it an unsigned number and you will get your answer. In most
languages that do not have a boolean type and they use a integer type
(regardless of size) they do it that way 0 = false max(int) = true. It
so happens that Max(int) in an signed type is always -1. But this
should be a problem with fpc and the AsBoolean property not sqlite's
fault. Some one decided that the C default of -1 for true instead of
the pascal's 1 is more appropriate for sqlite for some reason I
suppose.

----------------------------------------------------------------------
rvk · 2015-06-12 13:43

Quote from: taazz on June 12, 2015, 03:19:19 pmSome one decided that
the C default of -1 for true instead of the pascal's 1 is more
appropriate for sqlite for some reason I suppose.
So that someone was obviously wrong (in SQLite's case). According to
the documentation of SQLite it should always be 0 or 1. I understand
it could be different in other database-engine. So changing it in
fields.inc (for all engines) might not be a good idea. When the
connection with SQLite was written, the SetAsBoolean (and
GetAsBoolean) should have been overridden for SQLite to use 1 and 0
(instead of -1 and 0). Now the biggest problem is, that a lot of
databases written by FPC already have -1 in it because this is a
longstanding bug in the code. So changing it would cause problems too.

So if anybody comes across an existing correct database (with 1 for
true) they'll need to use a workaround to check for both 1 and -1
(where the -1 is written for true by FPC):
WHERE (Abc(bol) = :param_bol)

----------------------------------------------------------------------
LacaK · 2015-06-15 13:05

IMO problem is deeper in how boolean variants are handling.

Try this program:
Code: [Select]program bug_varBoolean;
{$APPTYPE CONSOLE}

uses
  SysUtils,
  Variants;

var
  b: boolean;
  wb: wordbool;
  v: variant;
  i: integer;

begin
  b:=True;
  v:=b;
  i:=v;
  writeln('varType=',varType(v),', value=', varToStr(v), ',
AsInteger=', i);
  if varType(v)=varBoolean then writeln('varBoolean');

  wb:=True;
  v:=wb;
  i:=v;
  writeln('varType=',varType(v),', value=', varToStr(v), ',
AsInteger=', i);
  if varType(v)=varBoolean then writeln('varBoolean');

  readln;
end.

As you see, when you assign "True" to variant, then this variant to
integer, you get "-1"
It is bit surprise for me 

And in TParam is value stored as variant, so when you bind boolean
parameter and later convert it to integer, you get -1.
It can be fixed  in TSQLite3Connection in method
TSQLite3Cursor.bindparams , but as you wrote it will break backward
compatibility.
No problem for me. IMO it is worth to fix it, with minor risk.

Btw in SQLite works: SELECT * FROM table WHERE 1
and also SELECT * FROM table WHERE -1 (same result)
SELECT * FROM table WHERE 0 (no rows)

So seems to me 0=False, other value=True ?

----------------------------------------------------------------------
rvk · 2015-06-15 13:42

Quote from: LacaK on June 15, 2015, 03:05:14 pmAs you see, when you
assign "True" to variant, then this variant to integer, you get "-1"
Yes, apparently variants have the same "problem" as wordbool.
Although, like taazz mentioned, this might be expected behavior
(because of the max(type) conversion) for booleans and variants and
like you say further on (0=False and other value=True). But when a
database dictates that 1 is true it should be followed as such
(regardless what any language would dictate as TRUE).

Quote from: LacaK on June 15, 2015, 03:05:14 pmSo seems to me 0=False,
other value=True ?
Yes, that's the correct definition I've found, too. Except for SQLite
(and maybe other DB-engines). When storing -1 as boolean (/integer) it
will not find it when checking for 1. So SELECT * FROM table WHERE
boolean_value = -1 will not give the records with 1 (and visa versa).
And that's a problem when you have an existing database with correct 1
values. Now for hardcoded SQL this wouldn't be a problem because you
could say "boolean_value <> 0" (like the definition in Lazarus for
boolean) but...

This problem would also exists for a prameter-query:
SELECT * FROM table WHERE boolean_value = :param
Although Lazarus might implement the boolean parameter as "(value <>
0)", the database engine has not. And the "WHERE boolean_value =
:param" is check on the server-side. So having 1 or -1 (or -99 or 99
for that matter) is a problem for database-engines because you can't
check for booleans with ONE parameterized statement. You would always
need to have 2 statements:
SELECT * FROM table WHERE boolean_value = 0
SELECT * FROM table WHERE boolean_value <> 0

Of course, the storing of 1 for TRUE should only be changed if you're
sure every database (which stores a boolean as an integer) does expect
a 1 (instead of a -1). Otherwise these changes should only be
implemented for the SQLite-library. (How many SQL-engines store an
integer for boolean value??)

Finally, for all those having multiple values (1 and -1) for boolean
already in their database, checking for ABS(boolean_value) <> 0 is the
safest course. So everyone using check booleans in databases with
"integer as boolean" should be aware of this change, so they can check
it like that (ABS(X)), too. Or they should run a "fix" on their
databases to change all the -1 back to the correct 1's.


(I didn't even mention other programs querying the database and
expecting 1 for TRUE)

----------------------------------------------------------------------
LuizAmérico · 2015-06-15 18:28

Technically is not difficult, in sqlite*dataset, it was changed to
store 1 sometime ago: http://svn.freepascal.org/cgi-
bin/viewvc.cgi?view=revision&revision=14441

----------------------------------------------------------------------
rvk · 2015-06-15 18:44

Quote from: LuizAmérico on June 15, 2015, 08:28:24 pmTechnically is
not difficult, in sqlite*dataset, it was changed to store 1 sometime
ago: http://svn.freepascal.org/cgi-
bin/viewvc.cgi?view=revision&revision=14441
I see the code is still there:
Code: [Select]   ftBoolean, ftWord:
      begin
        //ensure that boolean True value is stored as 1
        if Field.DataType = ftBoolean then
          TempStr := IfThen(Boolean(Buffer^), '1', '0')
        else
          Str(Word(Buffer^), TempStr);
        EditItem^.Row[FieldOffset] := StrAlloc(Length(TempStr) + 1);
        Move(PAnsiChar(TempStr)^, (EditItem^.Row[FieldOffset])^,
Length(TempStr) + 1);
      end; 

But why does it not store 1 for a boolean TRUE but -1 ??

O, wait. Yes, it's because I'm using TSQLQuery with a
TSQLite3Connection. And TSqlite3Dataset probably works correctly. Wow.
So TSqlite3Dataset stores -1 and TSQLQuery/TSQLite3Connection stores
1. Yeah, This definitely needs to be corrected

----------------------------------------------------------------------
taazz · 2015-06-15 20:36

Did you install any sqlite packages or you used the sqlquery and
sqlconnection components? If you used sqldb then the code you look is
irrelevant.

----------------------------------------------------------------------
rvk · 2015-06-15 20:54

Quote from: taazz on June 15, 2015, 10:36:20 pmDid you install any
sqlite packages or you used the sqlquery and sqlconnection components?
If you used sqldb then the code you look is irrelevant.
Yes, I already saw that.

I used the TSQLite3Connection from the sqldb-tab in combination with
TSQLQuery. That one stores -1 as boolean = TRUE in a SQLite3 database
(incorrectly) . I'm aware of the completely separate TSqlite3Dataset
(from which code I quoted).

My point is that if TSQLQuery stores a -1 for boolean = true and
TSqlite3Dataset stores a 1 (which is the correct one) you have two
components which do this differently and when switching from one to
the other you'll get really strange results if querying booleans.

Before you only had to contend with other program's maybe writing 1
(while TSQLQuery writes -1) but now you have 2 components within
Lazarus/FPC itself which handle the boolean differently. And the -1 of
TSQLQeury is incorrect (according to SQLite).

----------------------------------------------------------------------
taazz · 2015-06-15 21:26

Quote from: rvk on June 15, 2015, 10:54:46 pmQuote from: taazz on June
15, 2015, 10:36:20 pmDid you install any sqlite packages or you used
the sqlquery and sqlconnection components? If you used sqldb then the
code you look is irrelevant.
Yes, I already saw that.

I used the TSQLite3Connection from the sqldb-tab in combination with
TSQLQuery. That one stores -1 as boolean = TRUE in a SQLite3 database
(incorrectly) . I'm aware of the completely separate TSqlite3Dataset
(from which code I quoted).

My point is that if TSQLQuery stores a -1 for boolean = true and
TSqlite3Dataset stores a 1 (which is the correct one) you have two
components which do this differently and when switching from one to
the other you'll get really strange results if querying booleans.

Before you only had to contend with other program's maybe writing 1
(while TSQLQuery writes -1) but now you have 2 components within
Lazarus/FPC itself which handle the boolean differently. And the -1 of
TSQLQeury is incorrect (according to SQLite).
The solution is to enhance the existing tsqliteconnection to properly
support the boolean type. That is not very difficult to do I guess,
but I do not know the proper way to do it. I would guess that a custom
TSQLiteBooleanField class would do the trick its a bit elaborate but
it makes sense to have a custom field enforcing the value range than
to add any kind of case/if or rule on any other place I guess. I would
report a bug.

----------------------------------------------------------------------
rvk · 2015-06-15 21:38

Quote from: taazz on June 15, 2015, 11:26:40 pmThe solution is to
enhance the existing tsqliteconnection to properly support the boolean
type. That is not very difficult to do I guess, but I do not know the
proper way to do it. I would guess that a custom TSQLiteBooleanField
class would do the trick its a bit elaborate but it makes sense to
have a custom field enforcing the value range than to add any kind of
case/if or rule on any other place I guess. I would report a bug.
Yes, I'll report the bug, because I'm now sure it would need to be
addressed (especially because TSqlite3Dataset does have this fixed and
we can't have two different understanding of booleans for SQLite).

Before doing so I'll check if I can see what needs to be done
(learning experience for me ).

Are there any other DB-engines that have no real notion of TRUE and
FALSE and store boolean as an integer?

----------------------------------------------------------------------
eny · 2015-06-15 22:00

Quote from: rvk on June 15, 2015, 11:38:28 pmAre there any other DB-
engines that have no real notion of TRUE and FALSE and store boolean
as an integer?
SQL*Server

----------------------------------------------------------------------
rvk · 2015-06-15 22:07

Quote from: eny on June 16, 2015, 12:00:26 amSQL*Server
And where TRUE is described as 1 (one) and not "not 0"?

I know MySQL, for example, does also store an integer but in their
documentation it is stated that TRUE is if the integer is "not 0".

----------------------------------------------------------------------
LuizAmérico · 2015-06-16 01:43

See how i did in my sqliteconnection fork (i forgot about it):

https://code.google.com/p/luipack/source/detail?r=1059&path=/trunk/sql
ite/extensions/sqlite3slimconn.pp

----------------------------------------------------------------------
LacaK · 2015-06-16 06:42

Yes it is right solution. I would use:
        ftboolean:
checkerror(sqlite3_bind_int(fstatement,I,ord(p.AsBoolean)));
I will commit this and write comment to User changes.

There is no need touch others DB.

----------------------------------------------------------------------
LacaK · 2015-06-16 06:47

Quote from: rvk on June 15, 2015, 03:42:03 pmFinally, for all those
having multiple values (1 and -1) for boolean already in their
database, checking for ABS(boolean_value) <> 0 is the safest
course.mention other programs querying the database and expecting 1
for TRUE)

Why "ABS(boolean_value) <> 0" ?
isn't enough "boolean_value <> 0"

----------------------------------------------------------------------
rvk · 2015-06-16 07:11

Quote from: LacaK on June 16, 2015, 08:47:05 amWhy "ABS(boolean_value)
<> 0" ?
isn't enough "boolean_value <> 0"
Yeah, that was suposed to be: ABS(boolean_value) = 1

When using a parameterized query you want something like this:
SELECT * FROM table WHERE visible_for_me = :param
and
Query.Parambyname('visible_for_me').asBoolean := true; // or false;

When you have an existing database with values of 1 and -1 this
doesn't work in SQLite.
Using this it will work:
SELECT * FROM table WHERE ABS(visible_for_me) = :param

Because this bug existed a long time in SQLite-connection this
construction will still be necessary until all values are correct (or
you correct the wrong values in one time).

----------------------------------------------------------------------
LacaK · 2015-06-16 09:01

Good point. So it will break existing applications (and databases).
It makes patch bit problematic as it will require users change
existing SQL statements.

----------------------------------------------------------------------
rvk · 2015-06-16 09:07

Quote from: LacaK on June 16, 2015, 11:01:09 amGood point. So it will
break existing applications (and databases).
It makes patch bit problematic as it will require users change
existing SQL statements.
As it is now it's also breaks existing application if they use an
existing database with records filled by other programs or even with
the TSqlite3Dataset-component (because that one does write the 1
correctly).

So it's choosing the least bad option.

I, for one, now know of this incompatibility, so I will use
ABS(boolean)=:param in the future, but the longer this incompatibility
with SQLite-requirements exists, the harder it becomes to correct.

----------------------------------------------------------------------
madref · 2015-06-16 12:28

Wow....this is an interesting discussion.
And only because i asked you a question RVK

----------------------------------------------------------------------
rvk · 2015-06-16 12:53

Quote from: madref on June 16, 2015, 02:28:19 pmWow....this is an
interesting discussion.
And only because i asked you a question RVK
Yep, what a small question can lead to

----------------------------------------------------------------------
LacaK · 2015-06-17 12:36

I did commit patch in trunk: rev.31086
And comment here: http://wiki.freepascal.org/User_Changes_Trunk#DB

----------------------------------------------------------------------
rvk · 2015-06-17 12:49

Quote from: LacaK on June 17, 2015, 02:36:41 pmI did commit patch in
trunk: rev.31086
And comment here: http://wiki.freepascal.org/User_Changes_Trunk#DB
Yes, It's working. Thanks.

The comment is also clear. Let's hope others don't have problems with
it, otherwise we can direct them to that comments.

----------------------------------------------------------------------
LacaK · 2015-06-17 20:27

Quote from: rvk on June 17, 2015, 02:49:54 pmThe comment is also
clear. Let's hope others don't have problems with it, otherwise we can
direct them to that comments.
I am still considering add conditional compiler define, which will
allow relax old behavior.
Something like:
{$IFDEF SQLITE_OLD_BOOLEAN}
   // use P.AsInteger
{$ELSE}
  // use ord(P.AsBoolean)
{$ENDIF}
Not nice , but hard to say how many users will be affected with this
change and if they will be happier with such solution.

----------------------------------------------------------------------
taazz · 2015-06-17 20:38

Quote from: LacaK on June 17, 2015, 10:27:20 pmQuote from: rvk on June
17, 2015, 02:49:54 pmThe comment is also clear. Let's hope others
don't have problems with it, otherwise we can direct them to that
comments.
I am still considering add conditional compiler define, which will
allow relax old behavior.
Something like:
{$IFDEF SQLITE_OLD_BOOLEAN}
   // use P.AsInteger
{$ELSE}
  // use ord(P.AsBoolean)
{$ENDIF}
Not nice , but hard to say how many users will be affected with this
change and if they will be happier with such solution.
I wouldn't spend any more thought on the subject. The change is simple
enough and it shouldn't affect to many people. Although SQLite is one
of the most used data containers I would guess not many would use 1 or
0 and not true or false or even where X or where not X which I would
expect to work regardless of the value of boolean in the database.

Then again I'm not using sqlite not even for demos.

----------------------------------------------------------------------
rvk · 2015-06-17 21:14

Quote from: taazz on June 17, 2015, 10:38:43 pmAlthough SQLite is one
of the most used data containers I would guess not many would use 1 or
0 and not true or false or even where X or where not X which I would
expect to work regardless of the value of boolean in the database.
Well... the problem with SQLite is that you can't do something like
SELECT * FROM testtable WHERE bol = true
because SQLite does not know the term FALSE and TRUE.

Luckily you can use
SELECT * FROM testtable WHERE bol
and
SELECT * FROM testtable WHERE not bol
but the problem with that is you can't put it in a parameterized
statement.
And this doesn't work again:
SELECT * FROM testtable WHERE bol = :param_boolean

But you are correct. I think the impact to existing users will be
minimum and I think when they post the question about booleans in
SQLite we can point them in the right direction.

Quote from: LacaK on June 17, 2015, 10:27:20 pmI am still considering
add conditional compiler define, which will allow relax old behavior.
Not nice , but hard to say how many users will be affected with this
change and if they will be happier with such solution.
You could. But I think someone capable of recompiling FPC with this
conditional compiler define is also capable of correcting his/her own
program to act as it should according to SQLite-requirements. The
largest problem that I see is that they initially don't know about
this change and can stumble upon it and waste some time looking for a
solution. But hey... that's what we are here for

----------------------------------------------------------------------
avra · 2015-06-18 06:40

Quote from: rvk on June 15, 2015, 11:38:28 pmAre there any other DB-
engines that have no real notion of TRUE and FALSE and store boolean
as an integer?Firebird has native boolean type since v3, which is
still in beta. Interbase had the same problem and I don't know if it's
fixed yet.
http://www.firebirdfaq.org/faq12
http://www.firebirdnews.org/boolean-datatype-implemented-in-fb-3

----------------------------------------------------------------------
LacaK · 2015-06-18 08:47

Ok. Thanks for reply. So I will leave it as is. Hopping, that users
will read http://wiki.freepascal.org/User_Changes_Trunk#DB when they
will upgrade.

Btw. Interbase has already introduced native boolean datatype and it
is supported by IBConnection (together with Firebirds boolean)

----------------------------------------------------------------------
garlar27 · 2015-06-18 12:37

Well, is hard to believe that none which has tested their app wouldn't
have noted this problem.

Once I've been advised to do not use the boolean/logic field since it
was not portable to other RDBS, and to use this instead:
Code: [Select]  --   POSTGRES EXTRACT  --
  ENABLED character varying(1) NOT NULL DEFAULT '1'::character
varying,
  CONSTRAINT MYTABLE_ENABLED_CHECK CHECK (upper(ENABLED ::text) = ANY
(ARRAY['0'::text, '1'::text])),