Bug 62506 - Wrong implementation of truncation
Summary: Wrong implementation of truncation
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: 3.14-FINAL
Hardware: All All
: P2 major (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2018-06-28 14:49 UTC by duffy duck
Modified: 2020-10-20 18:06 UTC (History)
0 users

Example to reproduce the problem and patch (maven) (4.13 KB, application/zip)
2018-06-28 14:49 UTC, duffy duck

Note You need to log in before you can comment on or make changes to this bug.
Description duffy duck 2018-06-28 14:49:08 UTC
Created attachment 35998 [details]
Example to reproduce the problem and patch (maven)

We have a column with a formula which must truncate to 2 figures its result; the problem is that some values are truncated in a wrong way.
For example 0.29 is truncated to 0.28, 1.15 is truncated to 1.14 and so on. 

The reason of the error is due to Java binary representation of a Double.
To fix it we have used BigDecimal class.
Moreover now the behavior is different from MS Excel because the number of figures to truncate is not managed as an integer value. Also MS Excel accepts this number as double value but it manages the value as an integer. 

Attached there is a simple code example and a patch with our correction.

We have fixed the function TRUNC in the class NumericFunction, but the error is more generic and is also present in the simplest numerical calculations (for example 0.1 + 0.2 does not result in 0.3).

Executing the attached example we have as result:

0.1 + 0.2 = 0.30000000000000004  
0.28       -> TRUNC(number,2) ->               0.28      
0.29       -> TRUNC(number,2) ->               0.28
The same problem is present also in 3.17.

Comment 1 Dominik Stadler 2018-06-28 15:10:03 UTC
Thanks for the suggested patch, however using a BigDecimal for every round() calculation can cause performance degradation, is there a way to fix this without usage of BigDecimal objects?
Comment 2 duffy duck 2018-06-28 15:31:51 UTC
we have executed the following test:

with 10.000.000 iterations the execution time is changed from 800ms to 4s. 

Random r = new Random();
double result;
int loopNumber = 10000000;
long startTime = System.currentTimeMillis();
// 800 ms
for (int i=0; i<loopNumber; i++) {
   double d0 = r.nextDouble();
   double d1 = r.nextInt(5);
   double multi = Math.pow(10d, d1);
   if (d0 < 0)
      result = -Math.floor(-d0 * multi) / multi;
      result = Math.floor(d0 * multi) / multi;
long lapTime = System.currentTimeMillis();
System.out.println("Old calculation for " + loopNumber + " occurrence take " + (lapTime-startTime) +" ms");

// 4s
for (int i=0; i<loopNumber; i++) {
   double d0 = r.nextDouble();
   double d1 = r.nextInt(5);
   result = new BigDecimal(Double.toString(d0)).setScale((int)d1,BigDecimal.ROUND_DOWN).doubleValue();
long endTime = System.currentTimeMillis();
System.out.println("New calculation for " + loopNumber + " occurrence take " + (endTime-lapTime) +" ms");
Comment 3 PJ Fanning 2020-10-20 18:06:50 UTC
Should be fixed by https://github.com/apache/poi/pull/193