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

[Daniel Fuchs]

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/
>
> 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/
>>>
>>> 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
>>>
>>
>