shunping commented on PR #30758: URL: https://github.com/apache/beam/pull/30758#issuecomment-2150345644
> Do they equal when you invoke Object.equal method? If machine doesn't think its equal then there is a risk to change this. > > In general if there is no functionality issue of current status I usually hesitate to fix things as we've seen many seemingly harmless fix had unexpected breaking changes That sounds reasonable, Yi. I created a simple test, the lines commented below are the failed assertions. We can see that dt1 and dt2 are not considered equal when calling either `Objects.equals()` or `dt1.equals()`. ``` @Test public void testEquality() { Instant ist = Instant.now(); DateTime dt1 = new DateTime(ist.getMillis()); DateTime dt2 = new DateTime(ist); DateTime dt3 = ist.toDateTime(); System.out.println(dt1); System.out.println(dt2); System.out.println(dt3); // assertEquals(dt1, dt2); assertEquals(dt1, dt3); // assertTrue(Objects.equals(dt1, dt2)); assertTrue(Objects.equals(dt1, dt3)); // assertTrue(dt1.equals(dt2)); assertTrue(dt1.equals(dt3)); // assertSame(dt1, dt2); // assertSame(dt1, dt3); assertTrue(dt1.isEqual(dt2)); assertTrue(dt1.isEqual(dt3)); } ``` In order to make it consistent, we have to explicitly set timezone when calling the constructor with a `long` value. The following test passed. ``` @Test public void testEquality2() { Instant ist = Instant.now(); DateTime dt1New = new DateTime(ist.getMillis(), ist.getZone()); DateTime dt2 = new DateTime(ist); System.out.println(dt1New); System.out.println(dt2); assertEquals(dt1New, dt2); assertTrue(dt1New.equals(dt2)); assertTrue(Objects.equals(dt1New, dt2)); assertTrue(dt1New.isEqual(dt2)); } ``` In other words, `DateTime(Instant)` should be equivalent to `DateTime(Instant.getMills(), Instant.getZone())`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org