View Issue Details

IDProjectCategoryView StatusLast Update
0033755FPCPackagespublic2019-10-14 08:42
ReporterLacaKAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilitysometimes
Status closedResolutionfixed 
PlatformWin64OSOS Version
Product Version3.0.4Product Build 
Target VersionFixed in Version3.1.1 
Summary0033755: Update ODBC header function due to problem on Win64
DescriptionIn helper function in odbc headers is used cast from currency to int64.
However this cast behaves differently on Win64 and other platforms.
So patch uses different approach to overcome this.
TagsNo tags attached.
Fixed in Revision39160
FPCOldBugId0
FPCTarget
Attached Files
  • odbcsql.inc.diff (518 bytes)
    --- odbcsql.inc.ori	Thu May 17 13:43:50 2018
    +++ odbcsql.inc	Thu May 17 13:46:09 2018
    @@ -1909,7 +1909,7 @@
     end;
     
     function CurrToNumericStruct(c: currency): SQL_NUMERIC_STRUCT;
    -var n: int64; i: integer;
    +var n: int64 absolute c; i: integer;
     begin
       Result.precision:=18;
       Result.scale:=4;
    @@ -1919,7 +1919,7 @@
         Result.sign:=0;
         c := -c;
       end;
    -  n := NtoLE(int64(c));
    +  n := NtoLE(n);
       for i:=0 to high(Result.val) do begin
         Result.val[i] := n and $ff;
         n := n shr 8;
    
    odbcsql.inc.diff (518 bytes)

Relationships

related to 0033758 closedYuriy Sydorov Currency bug on Win64 only 
related to 0033439 resolvedFlorian Multiply x Currency x Win64 
related to 0028748 resolvedYuriy Sydorov Adding Double to Currency - wrong result 
related to 0036176 assignedFlorian Currency multiply by constant (divisible by 10000) on Win64 

Activities

LacaK

2018-05-18 08:36

developer  

odbcsql.inc.diff (518 bytes)
--- odbcsql.inc.ori	Thu May 17 13:43:50 2018
+++ odbcsql.inc	Thu May 17 13:46:09 2018
@@ -1909,7 +1909,7 @@
 end;
 
 function CurrToNumericStruct(c: currency): SQL_NUMERIC_STRUCT;
-var n: int64; i: integer;
+var n: int64 absolute c; i: integer;
 begin
   Result.precision:=18;
   Result.scale:=4;
@@ -1919,7 +1919,7 @@
     Result.sign:=0;
     c := -c;
   end;
-  n := NtoLE(int64(c));
+  n := NtoLE(n);
   for i:=0 to high(Result.val) do begin
     Result.val[i] := n and $ff;
     n := n shr 8;
odbcsql.inc.diff (518 bytes)

Michael Van Canneyt

2018-05-18 09:12

administrator   ~0108386

Last edited: 2018-05-18 09:12

View 2 revisions

Using an absolute is not acceptable, since it relies on the internal structure of a currency. This is supposed to be an opaque type. So I will not apply this patch

What is wrong with using
  n:=trunc(c*10000)
?

LacaK

2018-05-18 13:30

developer   ~0108405

Unfortunately, when I use:
  n := NtoLE(Trunc(c*10000));

Test reports:

<test name="TTestFieldTypes.TestCurrencyParamQuery">
<error ExceptionClassName="EODBCException">
<message>Could not execute statement. ODBC error details: LastReturnCode: SQL_ERROR; Record 1: SqlState: 42000; NativeError: 8023; Message: [Microsoft][ODBC Driver 11 for SQL Server][SQL Server]The incoming tabular data stream (TDS) remote procedure call (RPC) protocol stream is incorrect. Parameter 2 (""): The supplied value is not a valid instance of data type numeric. Check the source data for invalid values. An example of an invalid value is data of numeric type with scale greater than precision.;</message>
<sourceunit></sourceunit>
<methodname></methodname>
<linenumber>0</linenumber>
</error>

(I do not know why)

LacaK

2018-05-21 09:11

developer   ~0108460

Even if this situation (Trunc(c*10000)) is fixed in trunk according to repeating problems with currency data type in the past I think, that my original patch is good solution. It works with all versions of FPC and I do not thing, that sometime in the future it will not work (as I do not expect that currency will be sometimes changed to something other that int64; if this happens (it will be breaking change anyway) it is easy to fix and also other places in RTL will have to be changed)

Yuriy Sydorov

2018-05-21 11:19

manager   ~0108466

Since the "Trunc(c*10000)" issue has been fixed in trunk, the absolute hack should be targeted only for FPC 3.0.x and "Trunc(c*10000)" should be used otherwise.
Use {$idfef VER3_0} to target a code for FPC 3.0.x.

Marco van de Voort

2018-05-23 15:04

manager   ~0108496

Or simply apply direct to the fixes branch? If it contains no code for trunk, there is no reason to commit it to trunk.

Yes, the absolute is ugly, but it might not even be in a release version ever.

Yuriy Sydorov

2018-05-23 16:05

manager   ~0108497

I've found that the culprit is the "int64(currency)" typecast. It behaves differently for i386 and other CPUs.
For i386 it returns currency*1000, for others only the integer part of the currency is returned.
Delphi 7 prohibits the "int64(currency)" typecast at all.

It is needed to decide what to do. I suppose it is better fix the typecast for non-i386 CPUs, since for i386 this typecast behave this way for ages.

Michael Van Canneyt

2018-06-02 13:05

administrator   ~0108639

Used Trunc(c*10000).
Not using ifdefs. This code must work with any version of FPC.

Issue History

Date Modified Username Field Change
2018-05-18 08:36 LacaK New Issue
2018-05-18 08:36 LacaK File Added: odbcsql.inc.diff
2018-05-18 09:10 Michael Van Canneyt Assigned To => Michael Van Canneyt
2018-05-18 09:10 Michael Van Canneyt Status new => assigned
2018-05-18 09:12 Michael Van Canneyt Note Added: 0108386
2018-05-18 09:12 Michael Van Canneyt Status assigned => feedback
2018-05-18 09:12 Michael Van Canneyt Note Edited: 0108386 View Revisions
2018-05-18 13:30 LacaK Note Added: 0108405
2018-05-18 13:30 LacaK Status feedback => assigned
2018-05-18 13:59 LacaK Relationship added related to 0033758
2018-05-21 09:04 LacaK Relationship added related to 0033439
2018-05-21 09:05 LacaK Relationship added related to 0028748
2018-05-21 09:11 LacaK Note Added: 0108460
2018-05-21 11:19 Yuriy Sydorov Note Added: 0108466
2018-05-23 15:04 Marco van de Voort Note Added: 0108496
2018-05-23 16:05 Yuriy Sydorov Note Added: 0108497
2018-06-02 13:05 Michael Van Canneyt Fixed in Revision => 39160
2018-06-02 13:05 Michael Van Canneyt Note Added: 0108639
2018-06-02 13:05 Michael Van Canneyt Status assigned => resolved
2018-06-02 13:05 Michael Van Canneyt Fixed in Version => 3.1.1
2018-06-02 13:05 Michael Van Canneyt Resolution open => fixed
2018-06-04 08:03 LacaK Status resolved => closed
2019-10-14 08:42 LacaK Relationship added related to 0036176