[bug_rejected] Boolean Parameter in sqlite query

[guillaume]

file : dbc\ZDbcSqLiteStatement.pas
function : TZSQLitePreparedStatement.PrepareSQLParam(...)
line : 333

Code: Select all

if SoftVarManager.GetAsBoolean(Value) then Result := '''Y'''
else Result := '''N''';

Change to :

Code: Select all

if SoftVarManager.GetAsBoolean(Value) then Result := '1'
else Result := '0';

[mdaems]

Can you explain why this is a bug? I think it's just a matter of choice, but I can't be sure of that as your proposal doesn't tell a lot about what you want to achieve.
Or does SQLite define a Boolean field type you should set using 0 and 1? If you have to implement booleans yourself, you can do it also using the Y/N syntax.

So, if you want to have this changed the way you proposed, please show us current version is WRONG and why. If you have an other way to fix INCONVENIENT behaviour without breaking programs of other users, please tell us.

Mark

[aducom]

In the Delphi components there's a property called 'truth value' in which you can define the boolean value to be treated. SQLite is typeless so the boolean value needs to be emulated like any other field. This is a matter of a deal and not a bug.

[mdaems]

Hi,

This could be solved using connection/query properties and maybe a default in zeos.inc. Add a property string "bool_values='0#1'" and use that to define the way booleans are treated for the connection/query.
Priority chain : Query propery->if absent Connection property ->if absent Zeos.inc const.
I used # , but a comma or semicolon would do equally well. Jokes like 'Yes, you are lucky#No : what a pitty' wouldn't work, however.
Other things to think about :
- parsing this properties every time is slow, so try to avoid it eg by caching the truth value.
- try to implement it on the most abstract level. So the implementation can be easily ported to other databases.

Final question, who's doing this? If you send me the patch and it's good enough I want to commit it after some basic tests. It probably won't make it into version 6.6, but 6.7 must be possible.
Aducom, do you think you can do it?

I move the topic to the feature requests.

Mark

[aducom]

Will look into it.

albert

[Runspect]

To use 0/1 as Boolean value is better. Please, read this:

SQLite Users

Best Regards.

[aducom]

Point is, is it the best solution to all databases? Just to save a byte?

[salvois]

In SQLite

SELECT * FROM my_table WHERE my_bool_column

and

SELECT * FROM my_table WHERE NOT my_bool_column

don't work with the current Zeos approach of 'Y/N' (nor 'true/false' for the matter). 0/1 is needed for this. This is also consistent with other DBMSs such as Postgres and MySQL.
Please consider applying the fix.

Please also keep in mind that sharing a SQLite database between programs written in different languages is possible. Other programs should not be forced to use Zeos conventions (see also how blobs are encoded).

To fix the problem (I really feel it's a problem) without breaking too much existing applications, you could *accept* y/y,true/false,t/f,1/0 when *reading* from DB, but use 1/0 when writing. Or just let be it configurable at compile time.

[aducom]

AFAIK there's no real approach about how the true/false data needs to be stored in any database. However it isn't that difficult to use strtobool and bool tostr because of the standard vars in Delphi: TrueBoolStrs and FalseBoolStrs. Both are of type stringarray and you fill it with the different true-and-false data. From the helpfile:

var TrueBoolStrs: array of String;

Description

TrueBoolStrs is an array of strings that are used in conversions between boolean values and strings. When using the BoolToStr function, True values are converted to the first string in the list. When using the StrToBool function, any string in the list is converted to True.

[salvois]

I'm afraid I've not explained clearly enough.

There is no problem with the way the VCL interacts with "booleans" with either definition, there is no need for StrToBool and BoolToStr here. This is true as long as Delphi and the VCL are your only reference environment, and that you code for a single database backend. Please do not assume this is always the case. Interoperability is important, IMHO.

I talk about raw SQL. Please read again the example queries in my last message: they are obvious and portable across several database engines. I'd need to change all my SQL using booleans to include 'Y' and 'N' as rvalues. As SQLite doesn't support the 'true' and 'false' standard tokens, zero/nonzero is the most portable alternative.

As a side note, I see in the sources that what I proposed in my last message (last paragraph) is already implemented (borrowed from the Postgres driver).

[mdaems]

As a side note, I see in the sources that what I proposed in my last message (last paragraph) is already implemented (borrowed from the Postgres driver).

What's the problem, then?
Can you provide a fix that enables you to set this at compile (or even run) time? I can accept the patch when default = current setting.

Mark

[salvois]

The fix is the one proposed by the original poster. And, just to clarify, I've already it in my working copy for some time and I'm fine with it. You may sandwich it in IFDEFs or using some runtime parameter.
The point is: the current convention does not allow to formulate generic SQL, but enforce 'Y' and 'N' rvalues.
Please note that another interoperatibility issue is on blobs, but this is not trivial to change (the Zeos SQLite driver assumes null terminated values). More on this on a separate feature request, later.

[mdaems]

Can you please try this patch :

Code: Select all

Index: ZDbcSqLiteResultSet.pas
===================================================================
--- ZDbcSqLiteResultSet.pas	(revision 374)
+++ ZDbcSqLiteResultSet.pas	(working copy)
@@ -80,6 +80,8 @@
     FColumnNames: PPChar;
     FColumnValues: PPChar;
     FPlainDriver: IZSQLitePlainDriver;
+    FBooleanTrue : String;
+    FBooleanFalse : String;
   protected
     procedure Open; override;
     procedure FreeHandle;
@@ -180,6 +182,9 @@
   Statement: IZStatement; SQL: string; Handle: Psqlite;
   StmtHandle: Psqlite_vm; ColumnCount: Integer; ColumnNames: PPChar;
   ColumnValues: PPChar);
+var
+  Tempstr : String;
+  CommaPos : integer;
 begin
   inherited Create(Statement, SQL, TZSQLiteResultSetMetadata.Create(
     Statement.GetConnection.GetMetadata, SQL, Self));
@@ -192,6 +197,30 @@
   FColumnNames := ColumnNames;
   FColumnValues := ColumnValues;
 
+  If Statement.GetConnection.GetParameters.Values['true_false_values']<>'' then
+    begin
+      Tempstr := Statement.GetConnection.GetParameters.Values['true_false_values'];
+      CommaPos := Pos(Tempstr,',');
+      If CommaPos > 0 then
+        begin
+          FBooleanTrue := Copy(TempStr,1,CommaPos);
+          FBooleanFalse := Copy(TempStr,CommaPos+1,Length(TempStr)-CommaPos-1);
+        end
+      else
+        begin
+          FBooleanTrue := TempStr;
+          FBooleanFalse := '';
+        end;
+      If (FBooleanTrue[1]='''') and (FBooleanTrue[Length(FBooleanTrue)]='''') then FBooleanTrue := Copy(FBooleanTrue,2,Length(FBooleanTrue)-2);
+      If (FBooleanFalse[1]='''') and (FBooleanFalse[Length(FBooleanFalse)]='''') then FBooleanFalse := Copy(FBooleanFalse,2,Length(FBooleanFalse)-2);
+    end
+  else
+    begin
+      FBooleanTrue := '';
+      FBooleanFalse := '';
+    end;
+
+
   Open;
 end;
 
@@ -389,8 +418,13 @@
   CheckColumnConvertion(ColumnIndex, stBoolean);
 {$ENDIF}
   Temp := UpperCase(GetPChar(ColumnIndex));
-  Result := (Temp = 'Y') or (Temp = 'YES') or (Temp = 'T') or
-    (Temp = 'TRUE') or (StrToIntDef(Temp, 0) <> 0);
+
+  If (FBooleanTrue = '') and (FBooleanFalse = '') then
+    // Connection parameter 'true_false_values' is not used
+    Result := (Temp = 'Y') or (Temp = 'YES') or (Temp = 'T') or
+      (Temp = 'TRUE') or (StrToIntDef(Temp, 0) <> 0)
+  else
+    Result := (Temp = FBooleanTrue); 
 end;
 
 {**
Index: ZDbcSqLiteStatement.pas
===================================================================
--- ZDbcSqLiteStatement.pas	(revision 374)
+++ ZDbcSqLiteStatement.pas	(working copy)
@@ -86,6 +86,8 @@
   private
     FHandle: Psqlite;
     FPlainDriver: IZSQLitePlainDriver;
+    FBooleanTrue : String;
+    FBooleanFalse : String;
   protected
     function CreateExecStatement: IZStatement; override;
     function GetEscapeString(const Value: string): string;
@@ -289,11 +291,34 @@
 }
 constructor TZSQLitePreparedStatement.Create(PlainDriver: IZSQLitePlainDriver;
   Connection: IZConnection; const SQL: string; Info: TStrings; Handle: Psqlite);
+var
+  Tempstr : String;
+  CommaPos : integer;
 begin
   inherited Create(Connection, SQL, Info);
   FHandle := Handle;
   FPlainDriver := PlainDriver;
   ResultSetType := rtForwardOnly;
+  If Connection.GetParameters.Values['true_false_values']<>'' then
+    begin
+      Tempstr := Connection.GetParameters.Values['true_false_values'];
+      CommaPos := Pos(Tempstr,',');
+      If CommaPos > 0 then
+        begin
+          FBooleanTrue := Copy(TempStr,1,CommaPos);
+          FBooleanFalse := Copy(TempStr,CommaPos+1,Length(TempStr)-CommaPos-1);
+        end
+      else
+        begin
+          FBooleanTrue := TempStr;
+          FBooleanFalse := ''''''; //quoted empty string
+        end;
+    end
+  Else
+    begin
+      FBooleanTrue :='''Y''';
+      FBooleanFalse := '''N''';
+    end;
 end;
 
 {**
@@ -337,8 +362,8 @@
   else begin
     case InParamTypes[ParamIndex] of
       stBoolean:
-        if SoftVarManager.GetAsBoolean(Value) then Result := '''Y'''
-        else Result := '''N''';
+        if SoftVarManager.GetAsBoolean(Value) then Result := FBooleanTrue
+        else Result := FBooleanFalse;
       stByte, stShort, stInteger, stLong, stBigDecimal, stFloat, stDouble:
         Result := SoftVarManager.GetAsString(Value);
       stString, stBytes:

This should allow to add a connection parameter like 'true_false_values=1,0' , indicating that all boolean values read/written using this connection should use 1 as true value and 0 as false value. Attention : when using string values you should quote them like true_false_values='A','B'

I must admit, it's completely untested code... So I won't commit until somebody confirms the new feature works and it still works the old way.

Mark