Time.at(time) != time

[doits]

Steps to reproduce

Whyle discussing rspec/rspec-rails#2301 we noticed the following:

t = Time.current
t == Time.at(t)
# => false

Shouldn't it be true?

And:

t = Time.current
t
# => Fri, 27 Mar 2020 10:23:58 CET +01:00

Time.at(t)
# => 2020-03-27 10:23:58 2857861/4194304 +0100

Timezone seems to be broken after Time.at. Maybe that is the reason why comparison is false above?

Is this something for Rails to fix or is that correct behavior?

System configuration

Rails version: 6.0.2.2
Ruby version: 2.7.0

[pirj]

Just for the context, the difference is in nsec:

t=Time.current
t.nsec # => 3882000
Time.at(t).nsec # => 3881931

This does't happen with Time.now:

t=Time.now
t.nsec # => 328689000
Time.at(t).nsec # => 328689000

The same applies to Rails 4.2 and earlier Rubies.

[lacostenycoder]

@pirj Illustrates why you don't get the expected true valued when comparing these two object. Also Time.now is core ruby, while Time.current is defined by ActiveSupport.

The question is, why exactly is this a problem for you? What is the the use case where you would do this? You use two different methods which instantiate two different object types as shown here:

current_time = Time.current
at_time_called_on_current_time = Time.at(current_time)

current_time.class
=> ActiveSupport::TimeWithZone < Object
at_time_called_on_current_time.class
=> Time < Object

When writing to the database for example, it's nearly impossible to write 2 records at the exact same nanosecond. Here's an example where I'll multithread and write 2 new users at the "exact same time", but not really.

threads = []
2.times do
  threads << Thread.new do |i|
    u = User.new.save(validate: false)
  end
end
threads.each { |thr| thr.join }

time_stamps = User.u.all.order(created_at: :desc).limit(2).pluck(:created_at)
   (0.5ms)  SELECT  "users"."created_at" FROM "users"  ORDER BY "users"."created_at" DESC LIMIT 2
=> [
    [0] Sun, 29 Mar 2020 20:39:14 EDT -04:00,
    [1] Sun, 29 Mar 2020 20:39:14 EDT -04:00
]

And here it appears those two records were created at the same time. But they were not!

time_stamps[0] == time_stamps[1]
=> false

But this is factually true as can be illustrated in Postgres for example:

SELECT  users.created_at FROM "users"  ORDER BY "users"."created_at" DESC LIMIT 2
test_development-# ;
         created_at
----------------------------
 2020-03-30 00:39:14.612592
 2020-03-30 00:39:14.610636

The point is, Time.now is core ruby, but Time.current is activerecord and needs to behave the way you would expect it to when dealing with objects in the database.

[pirj]

@lacostenycoder I understand what you mean, but I disagree with some of the statements.
For the start, some context on the issue rspec/rspec-rails#2301

1. On MySQL, which is one of the officially supported Rails databases, the default time precision for a migration create_table { t.timestamps } that would add the following to the schema:

    t.datetime "created_at"
    t.datetime "updated_at"

is seconds, and usec and nsec would return 0.
This doesn't automatically mean that ActiveSupport::TimeWithZone should follow that and have no respect for fractions of seconds.

2. Even though time values fetched from the database (or, as in our example - from Redis, as we're talking about ActiveJob/Sidekiq serialization) are AS::TWZ, they are also Time:

time = User.last.updated_at
time.class # => ActiveSupport::TimeWithZone
time.is_a? Time # => true

so I would expect it to work in the same way (even though it's delegation, not inheritance). Especially keeping in mind that Time from Ruby core has support for time zones conversion.

The problem, again, is the incorrect behavior of Time.at from Ruby core being passed an argument of type AS::TWZ impersonating itself as Time, but providing different input that results in incorrect nanoseconds and a lower digit of microseconds to be off by one.

The root cause of the issue might be this:

The lowest digits of to_f and nsec are different because IEEE 754 double is not accurate enough to represent the exact number of nanoseconds since the Epoch.

but I'm pretty sure it can be worked around to provide exact the same value on the double conversion of AS::TWZ just like it works for Time.

Side note 1 I personally strongly disagree with the solution to this problem introduced here #35713, especially in the part of rounding the values of all time objects found in the payload.

Side note 2 as per your example with threads, even if AS::TWZ would disregard milliseconds, it doesn't mean that two times would be the same, it only significantly increases the probability of them having the same time, as the whole second may flip.

[lacostenycoder]

@pirj good points. So what is the recommended approach to fixing this? I still don't see a use case here. But I did run into this problem recently which led to confusion when were expecting time stamps to match but got false when comparing objects. I had the idea that perhaps adding a configuration setting? Looks like this didn't fix the problem

I guess this is the problem

From: activesupport-4.2.11.1/lib/active_support/core_ext/time/calculations.rb @ line 30:
Owner: #<Class:Time>
Visibility: public
Number of lines: 3

def current
  ::Time.zone ? ::Time.zone.now : ::Time.now
end

[pirj]

Nice find @lacostenycoder ! Tests added along with the fix seem to be unsufficient to prevent this regression from happening.

[lacostenycoder]

@pirj any ideas on the best approach to take here?

[pirj]

Found this workaround. It seems that Time.at(t.to_r) works fine:

t = Time.current
t.nsec == Time.at(t.to_r).nsec # => true

There's a Ruby core spec that covers Time.at with a Rational:

    it "roundtrips a Rational produced by #to_r" do
      t = Time.now()
      t2 = Time.at(t.to_r)

      t2.should == t
      t2.usec.should == t.usec
      t2.nsec.should == t.nsec
    end

It seems to copy over the underlying time data with no modifications if the object passed as an argument to at is detected as Time (Ruby source):

    else if (IsTimeval(time)) {
	struct time_object *tobj, *tobj2;
        GetTimeval(time, tobj);
        t = time_new_timew(klass, tobj->timew);
	GetTimeval(t, tobj2);
        TZMODE_COPY(tobj2, tobj);
    }

Also, the spec promises that the value would be .to_r if possible (Ruby spec):

    describe "with an argument that responds to #to_r" do
      it "coerces using #to_r" do
        o = mock_numeric('rational')
        o.should_receive(:to_r).and_return(Rational(5, 2))
        Time.at(o).should == Time.at(Rational(5, 2))
      end
    end

I don't think this (5/2) gives enough certainty regarding precision though. 😕

There's another spec that is intended to compare the equality of the argument with the result of Time.at:

    it "creates a new time object with the value given by time" do
      t = Time.now
      Time.at(t).inspect.should == t.inspect
    end

but again, the precision is left aside, as inspect truncates milliseconds, not to say microseconds and nanoseconds:

Time.now.inspect # => "2020-03-30 17:28:56 +0000"

I'm not certain if an object of a type ActiveSupport::TimeWithZone being passed to IsTimeval (and rb_typeddata_is_kind_of underneath it) would return true or false, I hope this is similar to rb_typeddata_is_kind_of:

Time.current.kind_of? Time # => true

but the observation of mismatch in nanoseconds points that it's false and our time passes through a series of lossy transformations [1] [2]:

        timew = rb_time_magnify(v2w(num_exact(time)));
        t = time_new_timew(klass, timew);

v2w(VALUE v)
{
    if (RB_TYPE_P(v, T_RATIONAL)) {
        if (RRATIONAL(v)->den != LONG2FIX(1))
            return WIDEVAL_WRAP(v);
        v = RRATIONAL(v)->num;
    }

and frankly, I can't see any check to whether it responds to .to_r, just that RB_TYPE_P check that is documented as:

RB_TYPE_P(value, type)
Is value an internal type (T_NIL, T_FIXNUM, etc.)?

I lack Ruby internals knowledge, but have a strong suspicion that .to_r call was the case in older Rubies, but then vanished, and def to_r in AS::TWZ does nothing useful.

Wondering when t = Time.current; t.nsec == Time.at(t).nsec stopped working, maybe somewhere around Ruby 2.2 or 2.3?

[lacostenycoder]

@pirj excellent research!

[pirj]

Yet another thing that comes into play:

# Layers additional behavior on Time.at so that ActiveSupport::TimeWithZone and DateTime
    # instances can be used when called with a single argument
    def at_with_coercion(*args)
...
      time_or_number = args.first

      if time_or_number.is_a?(ActiveSupport::TimeWithZone) || time_or_number.is_a?(DateTime)
        at_without_coercion(time_or_number.to_f).getlocal
...
    end
    alias_method :at_without_coercion, :at
    alias_method :at, :at_with_coercion

So even though AS::TWZ is represented by a loseless Rational, it is .to_f'd and passed to the original Ruby core Time.at. This is where it loses precision:

t=Time.now
t.nsec == Time.at(t.to_f).nsec # => false
t.nsec # => 148269000
Time.at(t.to_f).nsec # => 148268938

Bingo!

Wondering if there are any implications to change it this way:

-        at_without_coercion(time_or_number.to_f).getlocal
+        at_without_coercion(time_or_number.to_r).getlocal

It seems that DateTime does not support .to_r, so maybe this would be better:

-      if time_or_number.is_a?(ActiveSupport::TimeWithZone) || time_or_number.is_a?(DateTime)
+      if time_or_number.is_a?(ActiveSupport::TimeWithZone)
+        at_without_coercion(time_or_number.to_r).getlocal
+      elsif time_or_number.is_a?(DateTime)
         at_without_coercion(time_or_number.to_f).getlocal
       else
         at_without_coercion(time_or_number)
       end

[pirj]

My suspicion regarding Ruby's false promise of converting .to_r if possible seems to have found confirmation:

T=Class.new do
  def respond_to?(m, *)
    puts m
    return true if m == :to_r
    super
  end
  def to_r
    @r
  end
  def initialize(r)
    @r = r
  end
end

Time.at(T.new(Rational(500000000, 3)))

to_r <= at least it tried! 😆 
to_int
TypeError: can't convert T into an exact number

🦆 😕 🤷‍♂

[pirj]

Filed https://bugs.ruby-lang.org/issues/17131

I intend to send a pull request to make use of .to_r and revert the changes made in #35713

[benkoshy]

@pirj Well done on the bug fix!

The battle is won and the spoils are yours. The cheque is in your hand, but you are yet to cash it?! Any chance you could push through the amendments you have suggested?

[eugeneius]

Fixed by #40448.