Comparing TimeWithZones to Dates returns incorrect results depending on your time zone

[davidbalbert]

Depending on the value of Time.zone, comparing a TimeWithZone to a Date might return incorrect (or at least inconsistent) results. These results are different than the result of comparing a DateTime to a Date.

Steps to reproduce

Consider the following code (automated test case available):

Time.zone = "America/New_York"

date = Date.current
datetime = date.to_datetime
time_with_zone = date.in_time_zone

datetime <= date
date >= datetime

time_with_zone <= date
date >= time_with_zone

Expected behavior

All comparisons return true in all time zones.

Actual behavior

In time zones that are less than UTC (e.g. "America/New_York"), the DateTime comparisons return true, but the TimeWithZone comparisons return false.

If you switched the direction of the comparisons (i.e. switch <= to >= and vice versa), I believe time zones greater than UTC would exhibit the error, though I haven't tested.

I believe what is happening is that at some point the Date gets converted to midnight UTC regardless of the value of Time.zone. Midnight in "America/New_York" (UTC-4) is greater than midnight UTC, and the comparison fails.

It's possible this behavior is by design. The Date to "time" coercion could be defined to mean midnight UTC on the given date regardless of the time zone of the time you're comparing it to, but I think that leads to pretty confusing behavior.

System configuration

Rails version: I tested on 5.1.4 and on master

Ruby version: 2.4.2

[lukesarnacki]

Hi @davidbalbert , you are right about the actual behaviour, but it is expected behaviour imo. And you are right that to_datetime converts to UTC. If you check both results in UTC you get:

datetime.utc
# => 2017-10-19 00:00:00 UTC

date.in_time_zone.utc
# => 2017-10-19 04:00:00 UTC

If you go level deeper, you are actually comparing following integers:

datetime.to_i
# => 1508371200
time_with_zone.to_i
# => 1508385600

I may be wrong, but Date#to_datetime is in Ruby, not ActiveSupport, so you shouldn't be expecting that it will use rails time. And back when it was in Rails, documentation was clear about UTC: https://apidock.com/rails/Date/to_datetime

Hope that helps.

[lukesarnacki]

One more thing: you should be always using zone methods in Rails unless you really want to use system time or UTC. And actually yes, it might be confusing, lots of people confuse Time.now/Date.today and Time.current/Date.current, while there is a fundamental difference. Once you know that former are Ruby methods and latter are ActiveSupport methods, it all makes sense. Maybe there should be a section about dates and time zones in Rails Guides?

[davidbalbert]

I think you have misread my bug report.

This issue is not with how to_datetime works. It is that comparing a Date with an ActiveSupport::TimeWithZone has confusing behavior.

Specifically: when comparing a Date and a TimeWithZone, the comparison returns different results depending on the time zone of the TimeWithZone. This is because the Date is automatically coerced into a timestamp at midnight UTC. I think the comparison behavior would be more intuitive if the Date were coerced into a timestamp with the same time zone as the TimeWithZone.

As a possibly clearer example, consider the following code:

ActiveSupport::TimeZone["America/New_York"].parse("12:00am 2017-10-19") <= Date.parse("2017-10-19") # => false

ActiveSupport::TimeZone["America/New_York"].parse("11:59pm 2017-10-18") <= Date.parse("2017-10-19") # => false

You would expect both lines to return true because you'd expect the comparison to take place in the time zone of the TimeWithZone.

Because the code to compare dates and time with zones is provided by ActiveSupport, I would expect it to be time zone aware.

[pixeltrix]

@davidbalbert actually, what's happening in your example that the TWZ instance is being converted to UTC Time instance and then the Active Support core extension Time#compare_with_coercion converts it to a DateTime instance. If it didn't do this then this is what you'd get:

>> Time.now <= Date.today
ArgumentError: comparison of Time with Date failed

The reason that the coercion was added was for compatibility between Time and DateTime because TWZ instances used to wrap either type - if we didn't then you would've gotten an error like this:

>> Time.now <= DateTime.now
ArgumentError: comparison of Time with DateTime failed

I'm more inclined to not coerce for comparing dates with times because behaviour will likely be dependent on which side of the operators the respective types fall. Also since Date doesn't have any zone information we can't tell the difference between Date.today and Date.currrent, e.g:

>> Time.zone = "Hawaii"
>> ENV["TZ"] = "Asia/Tokyo"
>> Date.today
=> Wed, 01 Nov 2017
>> Date.current
=> Tue, 31 Oct 2017

The latter Date is already adjusted for the app's timezone so any further adjustment would be incorrect. The only way to coerce it would be to convert the time to a date and then compare it, e.g:

>> Time.current.to_date <= Date.current
=> true

In your example the best comparison would be to use String#in_time_zone, e.g:

ActiveSupport::TimeZone["America/New_York"].parse("12:00am 2017-10-19") <= "2017-10-19".in_time_zone("America/New_York") # => true

Given that the application context is likely to change the behaviour desired I'm loathe to introduce any kind of breaking change unless we can clearly define what the correct behaviour should be.

[davidbalbert]

Sounds like there's not a ton of interest in fixing this, so I will close the issue. Thanks for looking into it.