EVALUATION
A very interesting bug... here's what I think happened.
In the fix for 4227983 (java.sql.Timestamp didn't overload compareTo, setTime
and getTime, delta 1.35, 2000/02/02, integrated into 1.4.0), two compareTo
methods were added to the Timestamp class, one taking a Timestamp and the other
taking an Object. The compareTo(Object) method was specified as follows:
* Compares this Timestamp to another Object. If the Object is
* a Date, this function behaves like compareTo(Timestamp).
* Otherwise, it throws aClassCastException (as Timestamps are
* comparable only to other Timestamps)
* ...
The intent of this change was to allow Timestamps to be compared to Dates.
A later fix, for 4631234 (java.sql.Timestamp.compareTo no longer accepts
java.util.Dates in 1.4, delta 1.46 2002/04/01, integrated into 1.4.1), revised
the specification of the compareTo(Object) method instead to say:
* Compares this <code>Timestamp</code> object to the given
* <code>Object</code>, which must be a <code>Timestamp</code>
* object. If the argument is not a <code>Timestamp</code> object,
* this method throws a <code>ClassCastException</code> object.
* (<code>Timestamp</code> objects are
* comparable only to other <code>Timestamp</code> objects.)
In other words, the intent of this change was to require a ClassCastException
to be thrown when comparing a Timestamp to a Date. Such behavior would cause
the submitter's code to fail.
The change for 4631234 did not modify any code. It appears to have been made
under the assumption that the existing code already behaved according to the
new specification. Due to the presence of a compareTo(Date) method in the Date
superclass, however, such comparisons were actually allowed, and so the
submitter's code actually worked... until recently.
The generification changes for Tiger (4964490) added the compareTo(Date) method
to Timestamp in order to preserve what was assumed (from the specification) to
be the current behavior; unfortunately the fact that the current behavior was
different from the specified behavior was not noticed at that time. That new
method is what is now causing the submitter's code to receive the (specified)
ClassCastException.
A reasonable fix for this bug would be to revise the specification of the
Timestamp.compareTo(Date) method to allow Timestamp to be comparable to Dates
once again, and to redefine that method as
public int compareTo(java.util.Date d) {
return d.compareTo(this);
}
(Of course the real problem here is that java.sql.Timestamp should not have
been made a subclass of java.util.Date in the first place, as explained in item
7 of Bloch's "Effective Java", but that's water under the bridge.)
-- ###@###.### 2004/9/16
I agree with the above comprehensive assesment, and I'll work with the JCK team
and 1.5.x team to get fix for this. A specific determination as to the most
ideal delivery vehicle still has to be determined, but likely Mustang at this
point.
###@###.### 2004-09-16
I am looking into fixing this bug.
The following lines will be getting removed from javadocs for the method
public int compareTo(java.util.Date o)
* @exception ClassCastException if the argument is not a
* <code>Timestamp</code> object
* @since 1.5
*/
// This forwarding method ensures that the compareTo(Date) method defined
// in java.util.Date is not invoked on a Timestamp
###@###.### 2005-05-02 05:25:12 GMT
1. I will be removing the above lines from mustang i.e. jdk 6.0 and will not be doing the changes in jdk5.0uX versions.
I hope that is fine.
###@###.### 2005-06-07 13:58:51 GMT
2. Also will change the method code to, in addition to above :-
public int compareTo(java.util.Date d) {
return - (d.compareTo(this));
}
###@###.### 2005-06-07 14:39:59 GMT
Since this is a regression and a binary incompatibility,
it is not unlikely that we will get requests for backports.
So I think a backport to a Tiger update must be considered.
The proposed solution is problematic:
java.util.Date d1 = new java.sql.Timestamp(...);
java.util.Date d2 = new java.sql.Timestamp(...);
d1.compareTo(d2); // stack overflow
See "Suggested Fix" section for an alternative solution.
###@###.### 2005-06-07 19:02:25 GMT
I propose the following fix, other fixes propose either don't work with 1.4.x and 1.5.x or if they work don't produce the same results.
Index: src/java/sql/Timestamp.java
(cvs diff -wu Timestamp.java)
===================================================================
RCS file: /m/jws/jdbc4.0/src/java/sql/Timestamp.java,v
retrieving revision 1.5
diff -w -u -r1.5 Timestamp.java
--- src/java/sql/Timestamp.java 18 Jul 2005 19:33:37 -0000 1.5
+++ src/java/sql/Timestamp.java 21 Jul 2005 11:13:21 -0000
@@ -462,14 +462,22 @@
* <code>Timestamp</code> object
* @since 1.5
*/
- // This forwarding method ensures that the compareTo(Date) method defined
- // in java.util.Date is not invoked on a Timestamp
public int compareTo(java.util.Date o) {
- return compareTo((Timestamp)o);
- }
+ if(o instanceof Timestamp) {
+ // When Timestamp instance compare it with a Timestamp
+ // Hence it is basically calling this.compareTo((Timestamp))o);
+ // Note typecasting is safe because o is instance of Timestamp
+ return compareTo((Timestamp)o);
+ } else {
+ // When Date doing a o.compareTo(this)
+ // will give wrong results.
+ Timestamp ts = new Timestamp(o.getTime());
+ return this.compareTo(ts);
+ }
+ }
static final long serialVersionUID = 2745179027874758501L;
The test cases against which I verified the fix are already as given here and as below.
/////////////// Test Case 1 ///////////////////////
import java.sql.Timestamp;
import java.util.Date;
public class Foo {
public static void main(String[] args) {
Date d = new Date();
Timestamp ts = new Timestamp(d.getTime());
System.out.println(ts.compareTo(d));
}
}
/////////////// Test Case 2 ///////////////////////
import java.sql.Timestamp;
import java.util.Date;
public class Foo1 {
public static void main(String[] args) {
Date d1 = new Date();
java.util.Date d1 = new java.sql.Timestamp(d.getTime());
java.util.Date d2 = new java.sql.Timestamp(d.getTime());
System.out.println("Value is : "+d1.compareTo(d2));
}
}
Except the comments(//This forwarding) which I am not sure can be backported, I think this fix of the code can be backported without problems, if required.
Please comment for the fix.
|
Java