View Issue Details

IDProjectCategoryView StatusLast Update
0032979FPCFCLpublic2018-01-15 12:06
ReporterOndrej PokornyAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.1.1Product Buildrevision 37897 
Target Version3.2.0Fixed in Version3.1.1 
Summary0032979: [fcl-db] TParam: ambiguous datetime format is used that fails when regional settings are used (MSSQL)
DescriptionMSSQL: If you change DATEFORMAT from ymd to e.g. dmy, TParam-queries are wrongly filled which causes the query to fail.
Steps To ReproduceSee this program:

program DBconn;

uses
  db, mssqlconn, sqldb, sysutils;

var
  xSQL: TMSSQLConnection;
  Q: TSQLQuery;
  T: TSQLTransaction;
begin
  Q := nil;
  T := nil;
  xSQL := TMSSQLConnection.Create(nil);
  try
    xSQL.HostName := 'your-connection';
    xSQL.DatabaseName := 'your-database';
    xSQL.Connected := True;

    Q := TSQLQuery.Create(nil);
    T := TSQLTransaction.Create(xSQL);
    T.DataBase := xSQL;
    Q.DataBase := xSQL;
    Q.Transaction := T;

    Q.SQL.Text := 'SET DATEFORMAT dmy';
    Q.ExecSQL;
    T.Commit;

    Q.SQL.Text := 'DROP TABLE mytest';
    Q.ExecSQL;

    Q.SQL.Text := 'CREATE TABLE mytest (mydate DATETIME NOT NULL)';
    Q.ExecSQL;

    Q.SQL.Text := 'INSERT INTO mytest (mydate) VALUES (:mydate)';
    Q.ParamByName('mydate').AsDateTime := 43025.819861111115;
    Q.ExecSQL; // FAIL HERE

    Q.SQL.Text := 'SELECT mydate from mytest';
    Q.Open;
    Writeln(Q.Fields[0].AsString);

    T.Commit;
  finally
    Q.Free;
    xSQL.Free;
  end;

  Readln;
end.

!!! The use of TParams MUST BE independent on user settings !!!
Additional InformationA possible patch included. It changes the datetime format to ISO yyyy-mm-ddThh:mm:ss.zzz, which is unambiguous in MSSQL. On the other hand, the 'yyyy-mm-dd hh:mm:ss.zzz' is ambiguous in MSSQL!!!

The ISO format is supported by Postgres as well - but I am not sure about all different DB, so maybe you may want to create a TSQLConnection-dependent datetime format.

!!! Note that FSQLFormatSettings are ignored anyway !!!
TagsNo tags attached.
Fixed in Revision37974
FPCOldBugId
FPCTarget
Attached Files
  • sqldb.pp.patch (1,754 bytes)
    Index: src/sqldb/sqldb.pp
    ===================================================================
    --- src/sqldb/sqldb.pp	(revision 37897)
    +++ src/sqldb/sqldb.pp	(working copy)
    @@ -1702,8 +1702,8 @@
       if (not assigned(Field)) or Field.IsNull then Result := 'Null'
       else case Field.DataType of
         ftString   : Result := QuotedStr(Field.AsString);
    -    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Field.AsDateTime,FSqlFormatSettings) + '''';
    -    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd hh:nn:ss.zzz',Field.AsDateTime,FSqlFormatSettings) + '''';
    +    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Field.AsDateTime) + '''';
    +    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd', Field.AsDateTime)+'T'+FormatDateTime('hh:nn:ss.zzz', Field.AsDateTime) + '''';
         ftTime     : Result := QuotedStr(TimeIntervalToString(Field.AsDateTime));
       else
         Result := Field.AsString;
    @@ -1718,9 +1718,9 @@
         ftMemo,
         ftFixedChar,
         ftString   : Result := QuotedStr(GetAsString(Param));
    -    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Param.AsDateTime,FSQLFormatSettings) + '''';
    +    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Param.AsDateTime) + '''';
         ftTime     : Result := QuotedStr(TimeIntervalToString(Param.AsDateTime));
    -    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd hh:nn:ss.zzz', Param.AsDateTime, FSQLFormatSettings) + '''';
    +    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd', Param.AsDateTime)+'T'+FormatDateTime('hh:nn:ss.zzz', Param.AsDateTime) + '''';
         ftCurrency,
         ftBcd      : Result := CurrToStr(Param.AsCurrency, FSQLFormatSettings);
         ftFloat    : Result := FloatToStr(Param.AsFloat, FSQLFormatSettings);
    
    sqldb.pp.patch (1,754 bytes)
  • mssql-datetime-02.patch (2,630 bytes)
    Index: packages/fcl-db/src/sqldb/mssql/mssqlconn.pp
    ===================================================================
    --- packages/fcl-db/src/sqldb/mssql/mssqlconn.pp	(revision 37897)
    +++ packages/fcl-db/src/sqldb/mssql/mssqlconn.pp	(working copy)
    @@ -72,6 +72,7 @@
       protected
         // Overrides from TSQLConnection
         function GetHandle:pointer; override;
    +    function GetAsSQLText(Field : TField) : string; overload; override;
         function GetAsSQLText(Param : TParam) : string; overload; override;
         function GetConnectionCharSet: string; override;
         // - Connect/disconnect
    @@ -218,7 +219,20 @@
       Result  :=0;
     end;
     
    +function IsBinary(const s: string): boolean;
    +var i: integer;
    +begin
    +  for i:=1 to length(s) do if s[i] < #9 then Exit(true);
    +  Exit(false);
    +end;
     
    +function StrToHex(const s: string): string;
    +begin
    +  setlength(Result, 2*length(s));
    +  BinToHex(PChar(s), PChar(Result), length(s));
    +end;
    +
    +
     { TDBLibCursor }
     
     procedure TDBLibCursor.Prepare(Buf: string; AParams: TParams);
    @@ -350,18 +364,31 @@
       Result:=FDBProc;
     end;
     
    +function TMSSQLConnection.GetAsSQLText(Field: TField): string;
    +begin
    +  if Assigned(Field) and not Field.IsNull then
    +    case Field.DataType of
    +      ftBoolean:
    +        if Field.AsBoolean then
    +          Result:='1'
    +        else
    +          Result:='0';
    +      ftDateTime : Result := '''' + FormatDateTime('yyyymmdd hh:nn:ss.zzz', Field.AsDateTime) + '''';
    +      ftString, ftFixedChar, ftMemo:
    +        //if IsBinary(Field.AsString) then
    +        //  Result := '0x' + StrToHex(Field.AsString)
    +        //else
    +        Result := 'N' + inherited GetAsSQLText(Field);
    +      ftBlob, ftBytes, ftVarBytes:
    +        Result := '0x' + StrToHex(Field.AsString);
    +      else
    +        Result := inherited GetAsSQLText(Field);
    +    end
    +  else
    +    Result := inherited GetAsSQLText(Field);
    +end;
    +
     function TMSSQLConnection.GetAsSQLText(Param: TParam): string;
    -  function IsBinary(const s: string): boolean;
    -  var i: integer;
    -  begin
    -    for i:=1 to length(s) do if s[i] < #9 then Exit(true);
    -    Exit(false);
    -  end;
    -  function StrToHex(const s: string): string;
    -  begin
    -    setlength(Result, 2*length(s));
    -    BinToHex(PChar(s), PChar(Result), length(s));
    -  end;
     begin
       if not Param.IsNull then
         case Param.DataType of
    @@ -370,6 +397,7 @@
               Result:='1'
             else
               Result:='0';
    +      ftDateTime : Result := '''' + FormatDateTime('yyyymmdd hh:nn:ss.zzz', Param.AsDateTime) + '''';
           ftString, ftFixedChar, ftMemo:
             //if IsBinary(Param.AsString) then
             //  Result := '0x' + StrToHex(Param.AsString)
    
    mssql-datetime-02.patch (2,630 bytes)

Activities

Ondrej Pokorny

2018-01-08 17:11

developer  

sqldb.pp.patch (1,754 bytes)
Index: src/sqldb/sqldb.pp
===================================================================
--- src/sqldb/sqldb.pp	(revision 37897)
+++ src/sqldb/sqldb.pp	(working copy)
@@ -1702,8 +1702,8 @@
   if (not assigned(Field)) or Field.IsNull then Result := 'Null'
   else case Field.DataType of
     ftString   : Result := QuotedStr(Field.AsString);
-    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Field.AsDateTime,FSqlFormatSettings) + '''';
-    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd hh:nn:ss.zzz',Field.AsDateTime,FSqlFormatSettings) + '''';
+    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Field.AsDateTime) + '''';
+    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd', Field.AsDateTime)+'T'+FormatDateTime('hh:nn:ss.zzz', Field.AsDateTime) + '''';
     ftTime     : Result := QuotedStr(TimeIntervalToString(Field.AsDateTime));
   else
     Result := Field.AsString;
@@ -1718,9 +1718,9 @@
     ftMemo,
     ftFixedChar,
     ftString   : Result := QuotedStr(GetAsString(Param));
-    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Param.AsDateTime,FSQLFormatSettings) + '''';
+    ftDate     : Result := '''' + FormatDateTime('yyyy-mm-dd',Param.AsDateTime) + '''';
     ftTime     : Result := QuotedStr(TimeIntervalToString(Param.AsDateTime));
-    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd hh:nn:ss.zzz', Param.AsDateTime, FSQLFormatSettings) + '''';
+    ftDateTime : Result := '''' + FormatDateTime('yyyy-mm-dd', Param.AsDateTime)+'T'+FormatDateTime('hh:nn:ss.zzz', Param.AsDateTime) + '''';
     ftCurrency,
     ftBcd      : Result := CurrToStr(Param.AsCurrency, FSQLFormatSettings);
     ftFloat    : Result := FloatToStr(Param.AsFloat, FSQLFormatSettings);
sqldb.pp.patch (1,754 bytes)

Ondrej Pokorny

2018-01-12 10:48

developer  

mssql-datetime-02.patch (2,630 bytes)
Index: packages/fcl-db/src/sqldb/mssql/mssqlconn.pp
===================================================================
--- packages/fcl-db/src/sqldb/mssql/mssqlconn.pp	(revision 37897)
+++ packages/fcl-db/src/sqldb/mssql/mssqlconn.pp	(working copy)
@@ -72,6 +72,7 @@
   protected
     // Overrides from TSQLConnection
     function GetHandle:pointer; override;
+    function GetAsSQLText(Field : TField) : string; overload; override;
     function GetAsSQLText(Param : TParam) : string; overload; override;
     function GetConnectionCharSet: string; override;
     // - Connect/disconnect
@@ -218,7 +219,20 @@
   Result  :=0;
 end;
 
+function IsBinary(const s: string): boolean;
+var i: integer;
+begin
+  for i:=1 to length(s) do if s[i] < #9 then Exit(true);
+  Exit(false);
+end;
 
+function StrToHex(const s: string): string;
+begin
+  setlength(Result, 2*length(s));
+  BinToHex(PChar(s), PChar(Result), length(s));
+end;
+
+
 { TDBLibCursor }
 
 procedure TDBLibCursor.Prepare(Buf: string; AParams: TParams);
@@ -350,18 +364,31 @@
   Result:=FDBProc;
 end;
 
+function TMSSQLConnection.GetAsSQLText(Field: TField): string;
+begin
+  if Assigned(Field) and not Field.IsNull then
+    case Field.DataType of
+      ftBoolean:
+        if Field.AsBoolean then
+          Result:='1'
+        else
+          Result:='0';
+      ftDateTime : Result := '''' + FormatDateTime('yyyymmdd hh:nn:ss.zzz', Field.AsDateTime) + '''';
+      ftString, ftFixedChar, ftMemo:
+        //if IsBinary(Field.AsString) then
+        //  Result := '0x' + StrToHex(Field.AsString)
+        //else
+        Result := 'N' + inherited GetAsSQLText(Field);
+      ftBlob, ftBytes, ftVarBytes:
+        Result := '0x' + StrToHex(Field.AsString);
+      else
+        Result := inherited GetAsSQLText(Field);
+    end
+  else
+    Result := inherited GetAsSQLText(Field);
+end;
+
 function TMSSQLConnection.GetAsSQLText(Param: TParam): string;
-  function IsBinary(const s: string): boolean;
-  var i: integer;
-  begin
-    for i:=1 to length(s) do if s[i] < #9 then Exit(true);
-    Exit(false);
-  end;
-  function StrToHex(const s: string): string;
-  begin
-    setlength(Result, 2*length(s));
-    BinToHex(PChar(s), PChar(Result), length(s));
-  end;
 begin
   if not Param.IsNull then
     case Param.DataType of
@@ -370,6 +397,7 @@
           Result:='1'
         else
           Result:='0';
+      ftDateTime : Result := '''' + FormatDateTime('yyyymmdd hh:nn:ss.zzz', Param.AsDateTime) + '''';
       ftString, ftFixedChar, ftMemo:
         //if IsBinary(Param.AsString) then
         //  Result := '0x' + StrToHex(Param.AsString)
mssql-datetime-02.patch (2,630 bytes)

Ondrej Pokorny

2018-01-12 10:52

developer   ~0105689

mssql-datetime-02.patch: a better patch included. It changes MSSQL only so it doesn't interfere with any other DBS.

In the second patch I use the "yyyymmdd hh:nn:ss.zzz" format which is also unambiguous in MSSQL and can be written as one format string (no need to concatenate date/time parts with T).

I also added formats from the GetAsSQLText TParam-overload to TField-overload. Feel free to delete them if they are not necessary.

LacaK

2018-01-12 15:20

developer   ~0105696

I have applied patch to sqldb.pp to use ISO-8601 format for DateTime literals, which is not locale dependant and supported by PostgreSQL,MSSQL,MySQL,Sqlite.
(AFAICS this format is not supported by Firebird, seems that Firegird strictly follows SQL standard: <unquoted timestamp string> ::= <unquoted date string> <space> <unquoted time string>)

I have tested it with PostgreSQL 9.3, MSSQL 2012, MySQL 5.6, Sqlite 3.x

If there are doubts I can revert patch and apply only to mssqlconn.pp
IMO for practical usage it is okay, but if we want strictly follow sql standard in sqldb then AFAISC sql standard is not conforming with ISO-8601 with "T" delimiter

Ondrej Pokorny

2018-01-12 15:33

developer   ~0105697

Yes, Firebird fails with the ISO notation. So you may need to override both overloads for TIBConnection.GetAsSQLText.

LacaK

2018-01-12 16:04

developer   ~0105700

But Firebird ibconnection.pp does not call GetAsSQLText so there is no need to override as far as it is protected method and cannot be used in public.
(and I do not think, that somebody in custom descendant of TIBConnection will use GetAsSqlText)

IMO only question is if we want be strict in sqldb.pp according to sql standard also when there is no known practical reason for it.

Ondrej Pokorny

2018-01-12 16:10

developer   ~0105701

> But Firebird ibconnection.pp does not call GetAsSQLText so there is no need to override as far as it is protected method and cannot be used in public.

OK, I didn't know this detail. In this case it's OK. (On the other hand it's not that much work to add the overrides anyway.)

> IMO only question is if we want be strict in sqldb.pp according to sql standard also when there is no known practical reason for it.

You decide, I am good with whatever solution that works :)

Michael Van Canneyt

2018-01-12 16:37

administrator   ~0105703

I think it is better to be strict.

LacaK

2018-01-12 18:17

developer   ~0105712

Ok, I will fix it and test on monday
(Btw what strange mess Microsoft introduced by not following sql standard, which is here 25 years;
and "the DATEFORMAT setting might be different for datetime and smalldatetime values than for date, datetime2 and datetimeoffset values"
https://technet.microsoft.com/en-us/library/ms180878(v=sql.105).aspx#StringLiteralDateandTimeFormats )

LacaK

2018-01-15 09:49

developer   ~0105834

Fixed in rev. 37974.
- sqldb.pp reverted to strict ANSI SQL format of datetime literals
- mssqlconn.pp added ISO 8601 format for datetime parameters

Btw in MS SQL "datetime" data type is affected by SET DATEFORMAT and SET LANGUAGE, but "datetime2" datatype is NOT! Unbelievable!

Michael Van Canneyt

2018-01-15 10:16

administrator   ~0105836

Fixed by Laco.

Ondrej Pokorny

2018-01-15 12:06

developer   ~0105845

Thank you!

Issue History

Date Modified Username Field Change
2018-01-08 17:11 Ondrej Pokorny New Issue
2018-01-08 17:11 Ondrej Pokorny File Added: sqldb.pp.patch
2018-01-12 08:35 Michael Van Canneyt Assigned To => Michael Van Canneyt
2018-01-12 08:35 Michael Van Canneyt Status new => assigned
2018-01-12 10:48 Ondrej Pokorny File Added: mssql-datetime-02.patch
2018-01-12 10:52 Ondrej Pokorny Note Added: 0105689
2018-01-12 15:20 LacaK Note Added: 0105696
2018-01-12 15:20 LacaK Status assigned => feedback
2018-01-12 15:33 Ondrej Pokorny Note Added: 0105697
2018-01-12 15:33 Ondrej Pokorny Status feedback => assigned
2018-01-12 16:04 LacaK Note Added: 0105700
2018-01-12 16:04 LacaK Status assigned => feedback
2018-01-12 16:10 Ondrej Pokorny Note Added: 0105701
2018-01-12 16:10 Ondrej Pokorny Status feedback => assigned
2018-01-12 16:37 Michael Van Canneyt Note Added: 0105703
2018-01-12 18:17 LacaK Note Added: 0105712
2018-01-15 09:49 LacaK Note Added: 0105834
2018-01-15 10:16 Michael Van Canneyt Fixed in Revision => 37974
2018-01-15 10:16 Michael Van Canneyt Note Added: 0105836
2018-01-15 10:16 Michael Van Canneyt Status assigned => resolved
2018-01-15 10:16 Michael Van Canneyt Fixed in Version => 3.1.1
2018-01-15 10:16 Michael Van Canneyt Resolution open => fixed
2018-01-15 10:16 Michael Van Canneyt Target Version => 3.2.0
2018-01-15 12:06 Ondrej Pokorny Note Added: 0105845
2018-01-15 12:06 Ondrej Pokorny Status resolved => closed