Issue Details (XML | Word | Printable)

Key: JDO-263
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Andy Jefferson
Reporter: Michelle Caisse
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
JDO

TestArrayCollections: Field ArrayOfBigDecimal12 is stored incorrectly

Created: 20/Dec/05 05:18 AM   Updated: 09/Jan/06 09:45 AM
Return to search
Component/s: tck2
Affects Version/s: None
Fix Version/s: None

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works jdo-263.patch 2006-01-05 03:34 AM Michelle Caisse 12 kB

Resolution Date: 07/Jan/06 05:36 PM


 Description  « Hide
test(org.apache.jdo.tck.models.fieldtypes.TestArrayCollections)javax.jdo.JDOUserException: Field "org.apache.jdo.tck.pc.fieldtypes.ArrayCollections.ArrayOfObject1" is declared as a reference type (interface/Object) but no implementation classes of "java.lang.Object" have been found!
at org.jpox.metadata.MetaDataUtils.getImplementationNamesForReferenceField(MetaDataUtils.java:594)
at org.jpox.store.rdbms.table.ColumnCreator.createColumnsForReferenceField(ColumnCreator.java:184)
at org.jpox.store.rdbms.table.ColumnCreator.createColumnsForField(ColumnCreator.java:296)
at org.jpox.store.rdbms.table.ColumnCreator.createColumns(ColumnCreator.java:95)
at org.jpox.store.rdbms.table.ArrayTable.initialize(ArrayTable.java:83)
at org.jpox.store.rdbms.RDBMSManager$ClassAdder.addClassTablesAndValidate(RDBMSManager.java:2404)
at org.jpox.store.rdbms.RDBMSManager$ClassAdder.run(RDBMSManager.java:2033)
at org.jpox.store.rdbms.RDBMSManager$MgmtTransaction.execute(RDBMSManager.java:1893)
at org.jpox.store.rdbms.RDBMSManager.addClasses(RDBMSManager.java:479)
at org.jpox.store.rdbms.RDBMSManager.addClass(RDBMSManager.java:493)
at org.jpox.store.rdbms.RDBMSManager.getDatastoreClass(RDBMSManager.java:766)
at org.jpox.state.StateManagerImpl.populateStrategyFields(StateManagerImpl.java:781)
at org.jpox.state.StateManagerImpl.<init>(StateManagerImpl.java:584)
at org.jpox.AbstractPersistenceManager.internalMakePersistent(AbstractPersistenceManager.java:1076)
at org.jpox.AbstractPersistenceManager.makePersistent(AbstractPersistenceManager.java:1131)
at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.runTest(TestArrayCollections.java:93)
at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.test(TestArrayCollections.java:69)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at org.apache.jdo.tck.JDO_Test.runBare(JDO_Test.java:204)
at org.apache.jdo.tck.util.BatchTestRunner.start(BatchTestRunner.java:120)
at org.apache.jdo.tck.util.BatchTestRunner.main(BatchTestRunner.java:95)

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Andy Jefferson added a comment - 21/Dec/05 01:03 AM
Changed JPOX to support this "element-type" attribute. You now have

test(org.apache.jdo.tck.models.fieldtypes.TestArrayCollections)java.lang.NullPointerException
    [java] at java.lang.System.arraycopy(Native Method)
    [java] at java.util.Vector.copyInto(Vector.java:166)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.setValues(TestArrayCollections.java:134)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.runTest(TestArrayCollections.java:96)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.test(TestArrayCollections.java:69)

Andy Jefferson made changes - 21/Dec/05 01:03 AM
Field Original Value New Value
Assignee Andy Jefferson [ andy ] Michelle Caisse [ mcaisse ]
Michelle Caisse added a comment - 05/Jan/06 03:34 AM
In checkValues(), added test for BigDecimal type. Also round persisted value to actual scale of observed value before testing. Two elements of the serialized field still fail to match:

    [java] There was 1 failure:
    [java] 1) test(org.apache.jdo.tck.models.fieldtypes.TestArrayCollections)junit.framework.AssertionFailedError: Assertion (TestArrayCollections) failed:
    [java] Expected and observed do not match!!
    [java] For element 12(2), expected = 55552323232.545454, actual = 55552323232.545456 .
    [java] For element 12(3), expected = 6456456.7543543534865785, actual = 6456456.7543543530628085 .
    [java] at org.apache.jdo.tck.JDO_Test.fail(JDO_Test.java:546)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.checkValues(TestArrayCollections.java:196)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.runTest(TestArrayCollections.java:124)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.test(TestArrayCollections.java:73)
    [java] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [java] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    [java] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    [java] at org.apache.jdo.tck.JDO_Test.runBare(JDO_Test.java:204)
    [java] at org.apache.jdo.tck.util.BatchTestRunner.start(BatchTestRunner.java:120)
    [java] at org.apache.jdo.tck.util.BatchTestRunner.main(BatchTestRunner.java:95)

Michelle Caisse made changes - 05/Jan/06 03:34 AM
Attachment jdo-263.patch [ 12321691 ]
Repository Revision Date User Message
ASF #366436 Fri Jan 06 05:56:45 UTC 2006 mcaisse JDO-263
Files Changed
MODIFY /db/jdo/trunk/tck20/test/java/org/apache/jdo/tck/models/fieldtypes/TestArrayCollections.java

Michelle Caisse made changes - 06/Jan/06 03:00 PM
Assignee Michelle Caisse [ mcaisse ] Andy Jefferson [ andy ]
Michelle Caisse added a comment - 06/Jan/06 03:00 PM
Fixed issue with test in revision 366436. Now test fails on serialized field ArrayOfBigDecimal12. Other fields are all okay.

Andy Jefferson added a comment - 06/Jan/06 03:48 PM
Hi Michelle,
looks much better now. Not sure what you're expecting from me on this one. The error is

test(org.apache.jdo.tck.models.fieldtypes.TestArrayCollections)junit.framework.AssertionFailedError: Assertion (TestArrayCollections) failed:
    [java] Expected and observed do not match!!
    [java] For element 12[0], expected = 2007908.54548, actual = 2007908.54548000008799135684967041015625 .
    [java] For element 12[1], expected = 0.544, actual = 0.544000000000000039079850466805510222911834716796875 .
    [java] For element 12[2], expected = 3002323232.545454, actual = 3002323232.5454540252685546875 .
    [java] For element 12[3], expected = 64564645656.78657, actual = 64564645656.78656768798828125 .
    [java] For element 12[4], expected = 4564565465.2342, actual = 4564565465.23419952392578125 .
    [java] at org.apache.jdo.tck.JDO_Test.fail(JDO_Test.java:546)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.checkValues(TestArrayCollections.java:194)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.runTest(TestArrayCollections.java:111)
    [java] at org.apache.jdo.tck.models.fieldtypes.TestArrayCollections.test(TestArrayCollections.java:73)

so we have a series of floating point numbers that were stored in the RDBMS and when retrieved have a different precision. We know that RDBMS don't preserve the precision exactly (as shown on other TCK tests). Does the TCK not have a method to perform rounding so that the numbers can be compared at the precision of the value that was input ? With all of the differences above, if the actual value is rounded up or down (depending on the number) they would be identical and the test would pass.

Michelle Caisse added a comment - 07/Jan/06 12:26 AM
Hi Andy,

Two comments on this: First, on all other tests where we encountered this issue before, it was sufficient to use the BigDecimal compareTo method to get a match. The values differed only in precision, not in value; the expected values were padded with zeroes to the stored precision. Why is the array case different? Second, I did try rounding the values to the precision of the expected value before doing the match. This is the code shown in the attached patch, but not checked in. In that case, in the second of the two values tested still had two elements that didn't match because the values differed within the precision of the expected value. (With the checked-in code, the test stops after failing with the first tested value.) The result with the patch code is shown in my comment of 04 Jan on this issue.

Michelle Caisse added a comment - 07/Jan/06 12:50 AM
And, third, why does only the serialized BigDecimal field fail? The other mappings of BigDecimal fields match.

Andy Jefferson added a comment - 07/Jan/06 12:56 AM
You say that your patch changes the results to those you show in 04 Jan. Well if I round the numbers (that current SVN gives and that I quoted) to the same precision as what was "expected" I get the exact same number as the expected. I only have to look at the numbers to do that. For example if I take 64564645656.78656768798828125 and round it to the same precision as the expected (64564645656.78657 - i.e 5 decimal places) then I get 64564645656.78657. Where does your number come from ?

Why it is different from other tests? I've no idea. I do know that storage of and retrieval of large floating point numbers is unreliable - see the issue about "LessThanEquals" and about Derby storing things in a seemingly odd way. The only difference here is that you have a serialised array of large floating point numbers, as opposed to a single column with a large floating point number in it. Consequently we have introduced serialisation, and arrays
Why does the serialised BigDecimal fail ? see above. It is serialised.

I still fail to see what JPOX is expected to do when the underlying datastore doesnt store floating points in a reliable way.

Michelle Caisse added a comment - 07/Jan/06 02:24 AM
My numbers come from the second set of test values, not shown in the current error message because the test fails before getting there.

This looks like a bug to me. It certainly could be a datastore bug, but I don't believe that it is a TCK bug. I think the test is appropriate.

Craig Russell added a comment - 07/Jan/06 06:53 AM
The way JPOX stores serialized data is found in org.jpox.store.rdbms.mapping. Blob2RDBMSMapping. The code that transforms BigDecimal[ ] for storage is:
                else if (value instanceof BigDecimal[ ])
                {
                    byte[ ] data = TypeConversionHelper.getByteArrayFromBigDecimalArray(value);
                    ((PreparedStatement) ps).setBytes(param, data);
                }

Looking at the code in JPOX org.jpox.util.TypeConversionHelper, we notice what appears to be a big problem.

    /**
     * Convert an instance of our value class into a byte[].
     *
     * @param value Object to be converted
     *
     * @return converted byte array
     */
    public static byte[] getByteArrayFromBigDecimalArray(Object value)
    {
        if (value == null)
        {
            return null;
        }

        BigDecimal[] a = (BigDecimal[]) value;
        double[] d = new double[a.length];

        for (int i=0; i < a.length; i++)
        {
            d[i] = a[i].doubleValue();
        }

        return getByteArrayFromDoubleArray(d);
    }

It appears that to serialize a BigDecimal[ ] it would be better to avoid converting the BigDecimal[ ] to a double[ ]. Converting to a double[ ] is where the precision is lost. Some other techique for serializing BigDecimal[ ] is needed here.

Craig Russell made changes - 07/Jan/06 06:55 AM
Summary TestArrayCollections: Field "org.apache.jdo.tck.pc.fieldtypes.ArrayCollections.ArrayOfObject1" is declared as a reference type (interface/Object) but no implementation classes of "java.lang.Object" have been found! TestArrayCollections: Field ArrayOfBigDecimal12 is stored incorrectly
Craig Russell added a comment - 07/Jan/06 08:16 AM
It looks like if the code in org.jpox.store.rdbms.mapping.Blob2RDBMSMapping is simply removed, then the default serialization will be used, and this will do the right thing.

Similarly for BigInteger[ ] that has the same issue.

Andy Jefferson added a comment - 07/Jan/06 04:38 PM
Craig, Michelle,
thanks for your investigation. Always nice to have new JPOX developers ;-).

I agree that the issue is of JPOX's way of storing serialised arrays - not sure what I was thinking of earlier, must be the effects of too much alcohol over the holidays :-)

Blob2RDBMSMapping isn't actually used for BigDecimal[] storage. We use LongVarBinaryRDBMSMapping (since Derby uses LONGVARBINARY, as it has no BLOB type) but that was doing the same as you observed anyway.

There are 2 ways to store an array (e.g byte[]) in a single (BLOB) column.
1. Java serialisation (where the field is tagged as "serialized").
2. Use a byte stream.
The latter is useful where you want to store something like a JPEG and want its datastore representation to be just that, without the leading and trailing parts that ObjectOutputStream includes. What was happening is that BigDecimal[] was going through the byte stream process (and was also losing precision as you stated - retaining only DECIMAL64 precision, whereas it required DECIMAL128) and also not actually being serialised - we've had a JPOX JIRA issue for a month or so to allow both methods as configurable.

Storage of a BigDecimal[] as a byte stream and retaining the precision was fixed last night by Erik.
I'll update JPOX CVS later today to use the serialisation method when the field is marked as serialised, and byte streams otherwise.

Andy Jefferson added a comment - 07/Jan/06 05:36 PM
Fixed in JPOX CVS - builds dated 08/01/2006 or later. :-)

Andy Jefferson made changes - 07/Jan/06 05:36 PM
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Craig Russell added a comment - 09/Jan/06 09:45 AM
It didn't appear that we were getting much traction on this issue so Michelle and I played JPOX developer for a day. Nice to hear our investigation was appreciated.

I think that for field types of byte[ ] it makes most sense to send it "as is" to the datastore. For all the other types it is probably best to use Java serialization instead of a proprietary serialization. This is most portable across JDO implementations (most won't use exactly the JPOX algorithm for serializing).

But I also agree that some users might want the more efficient JPOX serialization algorithm compared to Java serialization. So your feature request for JPOX still makes sense to me. And I agree with your solution "to use the serialisation method when the field is marked as serialised, and byte streams otherwise".