ORC-306 Correct pre-1970 timestamps that were off by one second. #220
Conversation
Fixes apache#220 Signed-off-by: Owen O'Malley <omalley@apache.org>
|
So the fix detects whether the Timestamp class has the bug or not and then either compensates or doesn't. It also ensures that the TimestampColumnVector.time values are set correctly with the milliseconds on the bottom. The test does more now. First of all, instead of going from 56.1000 to 56.1999 seconds incrementing by 0.0001, which doesn't reveal the bug, it goes from 56.0000 to 56.1999 seconds incrementing by 0.0002. It also checks the values from the TimestampColumnVector rather than comparing using Timestamp. |
| @@ -49,6 +50,15 @@ | |||
| * Factory for creating ORC tree readers. | |||
| */ | |||
| public class TreeReaderFactory { | |||
| // The current JDK has a bug where values of the form: | |||
| // YYYY-MM-DD HH:MM:SS.000X is off by a second for dates before 1970. | |||
There was a problem hiding this comment.
Can you please link the BUG # for future reference (can be done on commit)?
Also can we run the tests against JDK version with the bug on travis CI to trigger TIMSTAMP_BUG == true code path? The logic for adjusting millis look good to me.
| Timestamp ts1 = Timestamp.valueOf("1969-12-25 12:34:56.0001"); | ||
| Timestamp ts2 = Timestamp.valueOf("1969-12-25 12:34:56.0011"); | ||
| // Compare the seconds, which should be the same. | ||
| TIMESTAMP_BUG = ts1.getTime()/1000 != ts2.getTime()/1000; |
There was a problem hiding this comment.
I think in this case getSeconds() would return the same but millis would have changed as ts2 added a whole new millisecond to the time. isn't it?
There was a problem hiding this comment.
getTime() returns the milliseconds after 1970-01-01 00:00:00. So getTime()/1000 should return the seconds, which should be the same.
|
Here is another way to look at the problem with each timestamp's getTime() and getNanos(): Timestamp 1969-12-25 12:34:56.0 as -530704000 and 0 When we use getTime()/1000 the change happens between .000999999 and .001000000. If you don't think that Java will change, we could hard code the change to use 999_999 all of the time and not do the test. |
Fixes #220 Signed-off-by: Owen O'Malley <omalley@apache.org>
Fixes apache#220 Signed-off-by: Owen O'Malley <omalley@apache.org> (cherry picked from commit 6c4865a) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> Change-Id: I60e96cff5e459458065bc5419bc9d4b01da955d7
No description provided.