RFR JDK-5077522 : Duration.compare incorrect for some values

[huizhe wang]

Thanks again Daniel and Lance!

Joe

On 6/21/2014 3:32 AM, Lance @ Oracle wrote:
> Agree this is better and cleaner!
>
>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
> Andersen| Principal Member of Technical Staff | +1.781.442.2037 
> <tel:+1.781.442.2037>
> Oracle Java Engineering
> 1 Network Drive <x-apple-data-detectors://34/0>
> Burlington, MA 01803 <x-apple-data-detectors://34/0>
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
> Sent from my iPad
>
> On Jun 21, 2014, at 4:27 AM, Daniel Fuchs <daniel.fuchs at oracle.com 
> <mailto:daniel.fuchs at oracle.com>> wrote:
>
>> Thanks Joe!
>>
>> This is much cleaner indeed :-)
>>
>> -- daniel
>>
>> On 6/21/14 4:36 AM, huizhe wang wrote:
>>> Thanks Daniel, Lance.
>>>
>>> On 6/20/2014 3:02 AM, Daniel Fuchs wrote:
>>>> Hi Joe,
>>>>
>>>> Thanks for the detailed explanation.
>>>> It really helps reviewing the fix!
>>>
>>> Glad to know it helps. I thought this part of spec could be 
>>> unfamiliar to people.
>>>
>>>>
>>>> This looks reasonable to me. One minor nit is that you
>>>> could turn:
>>>>
>>>> 769         BigInteger maxintAsBigInteger = 
>>>> BigInteger.valueOf((long) Integer.MAX_VALUE);
>>>
>>> Good catch!  I was going to do so but then forgot.
>>>
>>> I also refactored the check method so that the checks can be done in 
>>> one loop: 24 lines of code instead of the original 170. I feel good :-)
>>>
>>> The other changes are purely clean-up and in one case, JavaDoc.
>>>
>>> http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ 
>>> <http://cr.openjdk.java.net/%7Ejoehw/jdk9/5077522/webrev/>
>>>
>>> Best regards,
>>> Joe
>>>
>>>
>>>>
>>>> into a static final constant in the class.
>>>>
>>>> best regards,
>>>>
>>>> -- daniel
>>>>
>>>> On 6/17/14 9:19 PM, huizhe wang wrote:
>>>>> Hi,
>>>>>
>>>>> This is a long time compatibility issue: Duration.compare returns 
>>>>> equal
>>>>> for INDETERMINATE relations defined in XML Schema standard
>>>>> (http://www.w3.org/TR/xmlschema-2/#duration-order) as listed in the
>>>>> following table:
>>>>>
>>>>>     Relation
>>>>> P*1Y*     > P*364D*     <> P*365D*         <> P*366D*     < P*367D*
>>>>> P*1M*     > P*27D*     <> P*28D*     <> P*29D*     <> P*30D*     <>
>>>>> P*31D*     < P*32D*
>>>>> P*5M*     > P*149D*     <> P*150D*     <> P*151D*     <> P*152D* 
>>>>>     <>
>>>>> P*153D*     < P*154D*
>>>>>
>>>>>
>>>>>
>>>>> The order-relation of two Duratoin values x and y is x < y iff s+x 
>>>>> < s+y
>>>>> for each qualified datetime s listed below:
>>>>>
>>>>>  * 1696-09-01T00:00:00Z
>>>>>  * 1697-02-01T00:00:00Z
>>>>>  * 1903-03-01T00:00:00Z
>>>>>  * 1903-07-01T00:00:00Z
>>>>>
>>>>>
>>>>> The original implementation used Unix epoch, that is, 00:00:00 UTC 
>>>>> on 1
>>>>> January 1970, as s in the above calculation which violated the above
>>>>> specification. A patch during JDK 6 development added correct
>>>>> implementation of the spec, but it was unfortunately added after the
>>>>> original calculation using Epoch time.
>>>>>
>>>>> *The fix to the issue therefore is simply removing the calculation 
>>>>> using
>>>>> Epoch time.* I also consolidated the tedious max field value 
>>>>> checks into
>>>>> a method called checkMaxValue.
>>>>>
>>>>> *Patch:*
>>>>> http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ 
>>>>> <http://cr.openjdk.java.net/%7Ejoehw/jdk9/5077522/webrev/>
>>>>>
>>>>> Test:
>>>>> testCompareWithInderterminateRelation: this is a copy of the JCK test
>>>>> that tests INDETERMINATE relations.
>>>>> testVerifyOtherRelations: this is added to verify edge cases, e.g. 
>>>>> +- 1
>>>>> second to the original test cases. For example, to the original test:
>>>>> PT525600M is P365D <> P1Y, I added "PT525599M59S", "<", "P1Y", and
>>>>> PT527040M -> P366D <> P1Y, "PT527040M1S", ">", "P1Y"
>>>>>
>>>>> Below is the test result:
>>>>> Comparing P1Y and P365D: INDETERMINATE
>>>>> Comparing P1Y and P366D: INDETERMINATE
>>>>> Comparing P1M and P28D: INDETERMINATE
>>>>> Comparing P1M and P29D: INDETERMINATE
>>>>> Comparing P1M and P30D: INDETERMINATE
>>>>> Comparing P1M and P31D: INDETERMINATE
>>>>> Comparing P5M and P150D: INDETERMINATE
>>>>> Comparing P5M and P151D: INDETERMINATE
>>>>> Comparing P5M and P152D: INDETERMINATE
>>>>> Comparing P5M and P153D: INDETERMINATE
>>>>> Comparing PT2419200S and P1M: INDETERMINATE
>>>>> Comparing PT2678400S and P1M: INDETERMINATE
>>>>> Comparing PT31536000S and P1Y: INDETERMINATE
>>>>> Comparing PT31622400S and P1Y: INDETERMINATE
>>>>> Comparing PT525600M and P1Y: INDETERMINATE
>>>>> Comparing PT527040M and P1Y: INDETERMINATE
>>>>> Comparing PT8760H and P1Y: INDETERMINATE
>>>>> Comparing PT8784H and P1Y: INDETERMINATE
>>>>> Comparing P365D and P1Y: INDETERMINATE
>>>>> Comparing P1Y and P364D: expected: GREATER actual: GREATER
>>>>> Comparing P1Y and P367D: expected: LESSER actual: LESSER
>>>>> Comparing P1Y2D and P366D: expected: GREATER actual: GREATER
>>>>> Comparing P1M and P27D: expected: GREATER actual: GREATER
>>>>> Comparing P1M and P32D: expected: LESSER actual: LESSER
>>>>> Comparing P1M and P31DT1H: expected: LESSER actual: LESSER
>>>>> Comparing P5M and P149D: expected: GREATER actual: GREATER
>>>>> Comparing P5M and P154D: expected: LESSER actual: LESSER
>>>>> Comparing P5M and P153DT1H: expected: LESSER actual: LESSER
>>>>> Comparing PT2419199S and P1M: expected: LESSER actual: LESSER
>>>>> Comparing PT2678401S and P1M: expected: GREATER actual: GREATER
>>>>> Comparing PT31535999S and P1Y: expected: LESSER actual: LESSER
>>>>> Comparing PT31622401S and P1Y: expected: GREATER actual: GREATER
>>>>> Comparing PT525599M59S and P1Y: expected: LESSER actual: LESSER
>>>>> Comparing PT527040M1S and P1Y: expected: GREATER actual: GREATER
>>>>> Comparing PT8759H59M59S and P1Y: expected: LESSER actual: LESSER
>>>>> Comparing PT8784H1S and P1Y: expected: GREATER actual: GREATER
>>>>>
>>>>> Number of tests passed: 36
>>>>> Number of tests failed: 0
>>>>>
>>>>> Thanks,
>>>>> Joe
>>>>>
>>>>
>>>
>>