JDOQL test NotEquals comparing floating point numbers

• Key: JDO-206
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Michael Bouschen
• Reporter: Andy Jefferson
• Component/s: tck2

Description

The current TCK test (carried over from JDO 1.0) for NotEquals, uses != operator on floating point numbers. This is not a good practice, and is unreliable. Its probably the case that the Equals test uses == on the same content, which also is not a good idea (as noted in the latest spec). These tests need reviewing and a reliable alternate strategy adopting

Comments

[Craig Russell]

Some tricks need to be used. This test compares a BigDecimal parameter to a floating point field in the database.

[Erik Bengtson]

Somehow derby is not storing correct the value. I found that derby stores (float)-234.23 as -234.22999572753906

Here is the stacktrace that points the place with the bug.

Thread [main] (Suspended)
Double.isNaN(double) line: 494
NumberDataType.normalizeDOUBLE(double) line: not available
SQLDouble.setValue(float) line: not available
EmbedPreparedStatement30(EmbedPreparedStatement).setFloat(int, float) line: not available
NewProxyPreparedStatement.setFloat(int, float) line: 729
FloatRDBMSMapping.setFloat(Object, int, float) line: 90
FloatMapping(SingleFieldMapping).setFloat(PersistenceManager, Object, int[], float) line: 245
ParameterSetter.storeFloatField(int, float) line: 94
StateManagerImpl.providedFloatField(PersistenceCapable, int, float) line: 2420
AllTypes.jdoProvideField(int) line: not available
AllTypes.jdoProvideFields(int[]) line: not available
StateManagerImpl.provideFields(int[], FieldManager) line: 2827
UpdateRequest.execute(StateManager) line: 224
ClassTable.update(StateManager, FieldMetaData[]) line: 2118
RDBMSManager(StoreManager).update(StateManager, int[]) line: 780
StateManagerImpl.flush() line: 4401
PersistenceManagerImpl(AbstractPersistenceManager).flush() line: 3044
NonmanagedTransaction.getConnection(boolean, boolean, boolean) line: 220
NonmanagedTransaction.getConnection(boolean, boolean) line: 189
PersistenceManagerImpl(AbstractPersistenceManager).getConnection(boolean, boolean) line: 321
InsertRequest.execute(StateManager) line: 168
ClassTable.insert(StateManager) line: 2058
RDBMSManager(StoreManager).insert(StateManager) line: 733
StateManagerImpl.internalMakePersistent() line: 3261
StateManagerImpl.makePersistent() line: 3234
PersistenceManagerImpl(AbstractPersistenceManager).internalMakePersistent(Object, FieldValues) line: 1061
PersistenceManagerImpl(AbstractPersistenceManager).makePersistent(Object) line: 1112
AllTypes.load(PersistenceManager) line: 269
NotEquals(ComparisonTests).verifyDataLoaded(PersistenceManager) line: 91
NotEquals.test() line: 184
NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method]
NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39
DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25
Method.invoke(Object, Object...) line: 585
NotEquals(TestCase).runTest() line: 154
NotEquals(JDO_Test).runBare() line: 204
TestResult$1.protect() line: 106
TestResult.runProtected(Test, Protectable) line: 124
TestResult.run(TestCase) line: 109
NotEquals(TestCase).run(TestResult) line: 118
TestSuite.runTest(Test, TestResult) line: 208
TestSuite.run(TestResult) line: 203
BatchTestRunner(TestRunner).doRun(Test, boolean) line: 116
BatchTestRunner(TestRunner).doRun(Test) line: 109
BatchTestRunner.run(Test) line: 80
BatchTestRunner.run(Class) line: 75
NotEquals.main(String[]) line: 177


This is the code that reproduces the issue with derby

import org.apache.derby.iapi.error.StandardException;
import org.apache.derby.iapi.types.NumberDataType;

public class FloatTest
{

    /**
     * @param args
     */
    public static void main(String[] args)
    {
        try
        {
            System.out.println(NumberDataType.normalizeDOUBLE((float)-234.23));
        }
        catch (StandardException e)
        {
            e.printStackTrace();
        }
    }

}

[Erik Bengtson]

I extended the test, and when the JVM converts (float)-234.23 it results in -234.22999572753906.

Is that a JVM bug?

import org.apache.derby.iapi.error.StandardException;
import org.apache.derby.iapi.types.NumberDataType;

public class FloatTest
{

    /**
     * @param args
     */
    public static void main(String[] args)
    {
        try
        {
            System.out.println(NumberDataType.normalizeDOUBLE((float)-234.23)); //prints -234.22999572753906
            float f =(float)-234.23;
            System.out.println(f); //prints -234.23
            System.out.println((double)f); //prints -234.22999572753906
        }
        catch (StandardException e)
        {
            e.printStackTrace();
        }
    }

}

[Erik Bengtson]

one more test, shows that the double converted back to float prints the expected value. More, it seems to be a "normal" floating point numbers behaviour.. In conclusion, I think it's a bug in derby that should not convert that value to double when storing.

See

http://www.cygnus-software.com/papers/comparingfloats/comparingfloats.htm
http://stevehollasch.com/cgindex/coding/ieeefloat.html



public class Test
{

    /**
     * @param args
     */
    public static void main(String[] args)
    {
        float f=(float)-234.23;
        double d=(double)f;
        float f1=(float)d;
        System.out.println(f); //prints -234.23
        System.out.println(d); // -234.22999572753906
        System.out.println(f1); // -234.23
    }

}

I agree with Erik that there is a problem with derby since it converts the float value to a double when storing. However, this test class is not intended to test storing of float or double values, instead it should test the != operator in JDOQL queries. Thus I propose to use a different float value that does not change when converting it to a double, e.g. -234.25 instead of -234.23.

Attached you find a patch for review including the above change. It also added a localSetUp method adding a tearDown class to the test classes Equality, NotEquals, LessThan, LessThanOrEqual, GreaterThan, and GreaterThanOrEqual. All these tests did not clean up after test execution. Please note, if you run these tests after applying the patch, then they will most probably fail, because a previous run w/o the patch did not clean up the database. You nee to run the tests again.
 

[Craig Russell]

Changes look good.

[Craig Russell]

The issue here is that Derby treats the FLOAT data type is used to represent both single- and double-precision floating point numbers. To get a single-precision value in the database, you have to use either REAL data type or specify a length on FLOAT.

Everywhere we really want a 32-bit floating point number to be stored, we need to specify REAL or FLOAT(24) as the column type.

[Michael Bouschen]

Attached you find a new patch (JDO-206.patch2) for review.

The patch changes the schema for derby and uses the coumn type REAL for all float fields. With this change test NotEquals succeeds. I also fixed table PrimitiveTypes and changes the column type of doubleNull and doubleNotNull to DOUBLE.
Please note, the localSetUp change from JDO-206.patch is already checked in.

[Craig Russell]

The change looks good.

This sure had me puzzled for quite a while. The root cause of the issue was that the float values in the class were being mapped to FLOAT column types, which are double precision if no length is specified. Float values were converted to double values for both comparisons and storage, so these would work fine. That is, if a float value of e.g. 234.23 were stored, it would be stored as 234.22999572753906. A query with a float parameter of 234.23 would compare the converted double value 234.22999572753906 and it would work. But a query with a BigDecimal parameter of 234.23 would be converted to a double value of 234.2300000000000, which does not compare equal to the column value.

The change looks good to check in.

[Michael Bouschen]

Fixed with revision 358029.