Time/Datetime conversion inconsistency with MRI #4810

rdubya · 2017-10-10T14:40:35Z

Environment

Tested with JRuby 9.1.12.0 and JRuby 9.1.13.0 and compared to MRI 2.3.1

Expected Behavior

A Datetime value should be able to be converted to a Time and back to a Datetime and still retain the original value to match MRI's functionality

Actual Behavior

The value at the end does not match the value at the beginning

Example Cases

JRuby 9.1.13.0

jruby-9.1.13.0 :001 > datetime = DateTime.parse('-0001-12-31T23:58:59+00:00')
 => #<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
jruby-9.1.13.0 :002 > datetime.to_time
 => -0001-12-31 19:02:57 -0456
jruby-9.1.13.0 :003 > datetime.to_time.to_datetime
 => #<DateTime: 0000-01-02T19:02:57-04:56 ((1721059j,86339s,0n),-17762s,2299161j)>
jruby-9.1.13.0 :004 > time = Time.new(-1, 12, 31, 23, 59, 59, '+00:00')
 => -0001-12-31 23:59:59 UTC
jruby-9.1.13.0 :005 > time.to_datetime
 => #<DateTime: 0000-01-02T23:59:59+00:00 ((1721059j,86399s,0n),+0s,2299161j)>
jruby-9.1.13.0 :006 > time.to_datetime.to_time
 => 0000-01-02 19:03:57 -0456

MRI 2.3.1

2.3.1 :001 > require 'time'
 => true
2.3.1 :002 > datetime = DateTime.parse('-0001-12-31T23:58:59+00:00')
 => #<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
2.3.1 :003 > datetime.to_time
 => -0001-12-31 18:58:59 -0500
2.3.1 :004 > datetime.to_time.to_datetime
 => #<DateTime: -0001-12-31T18:58:59-05:00 ((1721057j,86339s,0n),-18000s,2299161j)>
2.3.1 :005 > time = Time.new(-1, 12, 31, 23, 59, 59, '+00:00')
 => -0001-12-31 23:59:59 +0000
2.3.1 :006 > time.to_datetime
 => #<DateTime: -0001-12-31T23:59:59+00:00 ((1721057j,86399s,0n),+0s,2299161j)>
2.3.1 :007 > time.to_datetime.to_time
 => -0001-12-31 18:59:59 -0500

The text was updated successfully, but these errors were encountered:

rdubya · 2017-10-10T14:57:54Z

Actually, I'm going to edit the title because I just verified that it isn't specific to BC times:

JRuby:

jruby-9.1.13.0 :001 > datetime = DateTime.parse('0003-12-31T23:58:59+00:00')
 => #<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
jruby-9.1.13.0 :002 > datetime.to_time
 => 0003-12-31 19:02:57 -0456
jruby-9.1.13.0 :003 > datetime.to_time.to_datetime
 => #<DateTime: 0004-01-02T19:02:57-04:56 ((1722520j,86339s,0n),-17762s,2299161j)>

MRI:

2.3.1 :001 > require 'time'
 => true
2.3.1 :002 > datetime = DateTime.parse('0003-12-31T23:58:59+00:00')
 => #<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
2.3.1 :003 > datetime.to_time
 => 0003-12-31 18:58:59 -0500
2.3.1 :004 > datetime.to_time.to_datetime
 => #<DateTime: 0003-12-31T18:58:59-05:00 ((1722518j,86339s,0n),-18000s,2299161j)>

rdubya · 2017-10-10T15:00:31Z

Sorry, I left out the Java version information:

java version "1.8.0_77"
Java(TM) SE Runtime Environment (build 1.8.0_77-b03)
Java HotSpot(TM) 64-Bit Server VM (build 25.77-b03, mixed mode)

rdubya · 2017-10-11T14:44:48Z

I just tracked down that the point where this quirkiness starts to happen is in the years 1883-1884. Newer years appear to work correctly, older years are converted incorrectly.

headius · 2017-10-30T14:17:37Z

This is likely a similar problem to #4822, where it appears we use different logic to resolve historical dates. I can reproduce locally, and changing the chronology we use seems to help but does not completely solve it.

Stock 9.1.14:

#<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
0003-12-31 18:08:23 -0550
#<DateTime: 0004-01-02T18:08:23-05:50 ((1722520j,86339s,0n),-21036s,2299161j)>

9.1.14 using ISO chronology in date.rb instead of Gregorian/Julian:

#<DateTime: 0003-12-29T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
0003-12-29 18:08:23 -0550
#<DateTime: 0003-12-29T18:08:23-05:50 ((1722518j,86339s,0n),-21036s,2299161j)>

headius · 2017-10-30T14:19:27Z

Note that the times in my last result are the same moment but have acquired a different timezone/offset.

headius · 2017-10-30T14:26:26Z

Oh I just realized that the last result from @rdubya also localizes the time zone, so it seems like the only fix needed is to use ISO chronology. The time zone localization was fixed in 2.4.

@eregon My patch is obviously just brute force. I assume you arrived at this collection of conditionals for a reason...can you offer some insight into how we should handle this and #4822?

eregon · 2017-10-30T15:00:05Z

I think the fix should be in Time#to_datetime, which seems buggy.
One simple way is to use DateTime.civil(year, month, day, h, min, s, offset), that should trivially fix it.
It seems Time#to_datetime creates a DateTime with the default calendar reform (ITALY), so nothing special to do there.

eregon · 2017-10-30T15:02:37Z

The #chronology helper method should be used for Date itself, but not for Time I think, since the chronology of a time is always fixed to the default GJ calendar with the italy reform.

headius · 2017-10-30T18:04:08Z

@eregon I'm a bit confused. You said:

I think the fix should be in Time#to_datetime, which seems buggy.

And then you said:

It seems Time#to_datetime creates a DateTime with the default calendar reform (ITALY), so nothing special to do there.

Is it buggy or is there nothing to do?

eregon · 2017-10-30T18:26:44Z

I meant there should be no need to mess with a chronology here, a Time converted to a DateTime just uses the default chronology.
Changing Time#to_datetime to use DateTime.civil(year, month, day, h, min, s, offset) should fix the problem.

headius · 2017-10-30T18:56:19Z

Ok, @eregon's suggested change does appear to fix #4822:

[--dev] ~/projects/jruby $ jruby time_bug_4822.rb 
1589-09-26 02:40:00 UTC
#<DateTime: 1589-09-26T02:40:00+00:00 ((2301699j,9600s,0n),+0s,2299161j)>
1573-11-22 01:46:40 UTC
#<DateTime: 1573-11-22T01:46:40+00:00 ((2295922j,6400s,0n),+0s,2299161j)>

[--dev] ~/projects/jruby $ /usr/bin/ruby time_bug_4822.rb 
1589-09-26 02:40:00 UTC
#<DateTime: 1589-09-26T02:40:00+00:00 ((2301699j,9600s,0n),+0s,2299161j)>
1573-11-22 01:46:40 UTC
#<DateTime: 1573-11-22T01:46:40+00:00 ((2295922j,6400s,0n),+0s,2299161j)>

But it still does not match MRI for this bug:

[--dev] ~/projects/jruby $ jruby -rdate -e "datetime = DateTime.parse('0003-12-31T23:58:59+00:00'); p datetime; p datetime.to_time; p datetime.to_time.to_datetime"
#<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
0003-12-31 18:08:23 -0550
#<DateTime: 0003-12-31T18:08:23-06:00 ((1722518j,86903s,0n),-21600s,2299161j)>

[--dev] ~/projects/jruby $ ~/.rubies/ruby-2.3.0/bin/ruby -rdate -e "datetime = DateTime.parse('0003-12-31T23:58:59+00:00'); p datetime; p datetime.to_time; p datetime.to_time.to_datetime"
#<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
0003-12-31 17:58:59 -0600
#<DateTime: 0003-12-31T17:58:59-06:00 ((1722518j,86339s,0n),-21600s,2299161j)>

eregon · 2017-10-30T19:01:07Z

@headius You should compare with MRI 2.4.0, before that MRI doesn't respect the timezone on DateTime#to_time.

eregon · 2017-10-30T19:04:00Z

Ah but this issue is for JRuby 9.1, nevermind then.

headius · 2017-10-30T19:04:28Z

@eregon But I'm fixing this for 9.1, which is compatible with 2.3.0. And the time doesn't match anyway.

eregon · 2017-10-30T19:12:18Z

Trying it locally I get:

$ ruby -v -rdate -e "datetime = DateTime.parse('0003-12-31T23:58:59+00:00'); p datetime; p datetime.to_time; p datetime.to_time.to_datetime"
jruby 9.1.13.0 (2.3.3) 2017-09-06 8e1c115 OpenJDK 64-Bit Server VM 25.131-b12 on 1.8.0_131-b12 +jit [linux-x86_64]

#<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
0004-01-01 01:04:20 +0105
#<DateTime: 0004-01-03T01:04:20+01:05 ((1722521j,-61s,0n),+3921s,2299161j)>
ruby -v -rdate -e   3.44s user 0.12s system 260% cpu 1.367 total

$ chruby 2.3.4          
$ ruby -v -rdate -e "datetime = DateTime.parse('0003-12-31T23:58:59+00:00'); p datetime; p datetime.to_time; p datetime.to_time.to_datetime"
ruby 2.3.4p301 (2017-03-30 revision 58214) [x86_64-linux]

#<DateTime: 0003-12-31T23:58:59+00:00 ((1722518j,86339s,0n),+0s,2299161j)>
0004-01-01 01:04:20 +0105
#<DateTime: 0004-01-01T01:04:20+01:05 ((1722518j,86339s,0n),+3921s,2299161j)>

So the Times do match at me.
But the Date do not, JRuby returns January 3rd.
There also seems to be something different for the zone offset.

eregon · 2017-10-30T19:14:23Z

(My example above is without patch)

eregon · 2017-10-30T19:16:29Z

Could MRI be rounding the timezone from -0550 to -0600 at you?
What's your TZ zone? ($ readlink /etc/localtime)?

headius · 2017-10-31T20:37:44Z

@eregon I'm in America/Chicago TZ.

Still unclear what the right fixes are. It would be nice to iron this out for 9.1.14.

eregon · 2017-11-03T18:21:57Z

This should be fixed by 8da4e66 and spec added in 6c7ccbe.
Also related, the same for Time#to_date: 36d6d58.
@headius I let you backport those to 9.1.

Both of these fixes use the original definition from date.rb in 1.9.2:
https://github.com/ruby/ruby/blob/ruby_1_9_2/lib/date.rb

Those methods were changed to optimize with Joda-DateTime but obviously the corner cases are wrong (e.g.: Joda throws an error when given a date which is in the middle of the reform) and there seems to be no simple solution, so I reverted to the original code.

rdubya · 2017-11-07T18:39:07Z

It looks like DateTime#to_time is still ending up with a weird timezone issue for the example above. It does look like these changes fixed #4822 though

headius · 2017-11-08T17:04:29Z

@rdubya JRuby appears to match expectations now. I think you may be seeing the difference between Ruby 2.4 (and later 2.3) that does not localize during to_time and 2.3 and earlier that still does.

Here's my results for JRuby 9.1 versus Ruby 2.3:

$ TZ=EST jruby -v time_bug_4810.rb 
jruby 9.1.14.0-SNAPSHOT (2.3.3) 2017-11-08 7e7a8f3 Java HotSpot(TM) 64-Bit Server VM 25.131-b11 on 1.8.0_131-b11 +jit [darwin-x86_64]
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 18:58:59 -0500
#<DateTime: -0001-12-31T18:58:59-05:00 ((1721057j,86339s,0n),-18000s,2299161j)>
-0001-12-31 23:59:59 UTC
#<DateTime: -0001-12-31T23:59:59+00:00 ((1721057j,86399s,0n),+0s,2299161j)>
-0001-12-31 18:59:59 -0500

$ (chruby 2.3.4 ; TZ=EST ruby -v time_bug_4810.rb)
ruby 2.3.4p301 (2017-03-30 revision 58214) [x86_64-darwin16]
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 18:58:59 -0500
#<DateTime: -0001-12-31T18:58:59-05:00 ((1721057j,86339s,0n),-18000s,2299161j)>
-0001-12-31 23:59:59 +0000
#<DateTime: -0001-12-31T23:59:59+00:00 ((1721057j,86399s,0n),+0s,2299161j)>
-0001-12-31 18:59:59 -0500

And JRuby master (9.2) versus Ruby 2.4:

$ TZ=EST jruby -v time_bug_4810.rb 
jruby 9.2.0.0-SNAPSHOT (2.4.1) 2017-11-08 2e10a4e Java HotSpot(TM) 64-Bit Server VM 25.131-b11 on 1.8.0_131-b11 +jit [darwin-x86_64]
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 23:58:59 UTC
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 23:59:59 UTC
#<DateTime: -0001-12-31T23:59:59+00:00 ((1721057j,86399s,0n),+0s,2299161j)>
-0001-12-31 23:59:59 UTC

$ (chruby 2.4.1 ; TZ=EST ruby -v time_bug_4810.rb)
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin16]
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 23:58:59 +0000
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 23:59:59 +0000
#<DateTime: -0001-12-31T23:59:59+00:00 ((1721057j,86399s,0n),+0s,2299161j)>
-0001-12-31 23:59:59 +0000

As you can see, the results match.

There is an oddity on my local system where the time zone is coming back as -0550 rather than -0600 (I am in US Central Time, America/Chicago) but I don't think it's related to this issue.

 $ jruby -v time_bug_4810.rb 
jruby 9.1.14.0-SNAPSHOT (2.3.3) 2017-11-08 7e7a8f3 Java HotSpot(TM) 64-Bit Server VM 25.131-b11 on 1.8.0_131-b11 +jit [darwin-x86_64]
#<DateTime: -0001-12-31T23:58:59+00:00 ((1721057j,86339s,0n),+0s,2299161j)>
-0001-12-31 18:08:23 -0550
#<DateTime: -0001-12-31T18:08:23-05:50 ((1721057j,86339s,0n),-21036s,2299161j)>
-0001-12-31 23:59:59 UTC
#<DateTime: -0001-12-31T23:59:59+00:00 ((1721057j,86399s,0n),+0s,2299161j)>
-0001-12-31 18:09:23 -0550

rdubya changed the title ~~Time/Datetime conversion inconsistency for BC date~~ Time/Datetime conversion inconsistency with MRI Oct 10, 2017

eregon mentioned this issue Nov 3, 2017

Time#to_datetime incorrect for times before the date of calendar reform #4822

Closed

headius closed this as completed Nov 8, 2017

headius added this to the JRuby 9.1.14.0 milestone Nov 8, 2017

headius added core stdlib labels Nov 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Time/Datetime conversion inconsistency with MRI #4810

Time/Datetime conversion inconsistency with MRI #4810

rdubya commented Oct 10, 2017

rdubya commented Oct 10, 2017

rdubya commented Oct 10, 2017

rdubya commented Oct 11, 2017

headius commented Oct 30, 2017

headius commented Oct 30, 2017

headius commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

headius commented Oct 30, 2017 •

edited

Loading

eregon commented Oct 30, 2017

headius commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

headius commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

headius commented Oct 31, 2017

eregon commented Nov 3, 2017 •

edited

Loading

rdubya commented Nov 7, 2017

headius commented Nov 8, 2017

Time/Datetime conversion inconsistency with MRI #4810

Time/Datetime conversion inconsistency with MRI #4810

Comments

rdubya commented Oct 10, 2017

Environment

Expected Behavior

Actual Behavior

Example Cases

rdubya commented Oct 10, 2017

rdubya commented Oct 10, 2017

rdubya commented Oct 11, 2017

headius commented Oct 30, 2017

headius commented Oct 30, 2017

headius commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

headius commented Oct 30, 2017 • edited Loading

eregon commented Oct 30, 2017

headius commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

headius commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

eregon commented Oct 30, 2017

headius commented Oct 31, 2017

eregon commented Nov 3, 2017 • edited Loading

rdubya commented Nov 7, 2017

headius commented Nov 8, 2017

headius commented Oct 30, 2017 •

edited

Loading

eregon commented Nov 3, 2017 •

edited

Loading