Awesome, thanks for the reply.

 - Ken

On Jan 15, 2010, at 10:24 AM, Jacob Lauemøller wrote:

> The problem only manifests itself if the sub-second part is stored with 4 or 
> more significant digits. So if you only store and retrieve milliseconds (and 
> thats the way I read your code) you should be home safe.
> 
> On 15/01/2010, at 15.56, Ken Collins wrote:
> 
>> 
>> I plan on taking a look at this too. I think I had to solve it in the SQL 
>> Server adapter in my own way since it only stores milliseconds. If you care 
>> to look at our code, here are a few key sections in the tests.
>> 
>> http://github.com/rails-sqlserver/2000-2005-adapter/blob/master/test/cases/adapter_test_sqlserver.rb#L271
>> http://github.com/rails-sqlserver/2000-2005-adapter/blob/master/lib/active_record/connection_adapters/sqlserver_adapter.rb#L328
>> 
>> 
>> 
>> On Jan 15, 2010, at 9:45 AM, Jacob Lauemøller wrote:
>> 
>>> Thanks for the response -- we use PostgreSQL which does store all six 
>>> microsecond digits.
>>> 
>>> Jacob
>>> 
>>> 
>>> On 15/01/2010, at 15.41, Chris Cruft wrote:
>>> 
>>>> I gave up on that kind of resolution when I found that MySQL doesn't
>>>> support it!  I'll try to test the patch though.
>>>> 
>>>> -Chris
>>>> 
>>>> On Jan 14, 10:20 am, Jacob Lauemøller <[email protected]>
>>>> wrote:
>>>>> Hi all,
>>>>> 
>>>>> The microsecond handling in 
>>>>> ActiveRecord::ConnectionAdapters::Column#fast_string_to_time and 
>>>>> ActiveRecord::ConnectionAdapters::Column#microseconds fail for some 
>>>>> values.
>>>>> 
>>>>> In slightly more than 1% of all possible 6-digit cases, writing a 
>>>>> timestamp to a database column and then reading it back in results in a 
>>>>> different value being returned to the program.
>>>>> 
>>>>> So, for instance, saving the timestamp
>>>>> 
>>>>>      2010-01-12 12:34:56.125014
>>>>> 
>>>>> and then loading it again from the database yields
>>>>> 
>>>>>      2010-01-12 12:34:56.125013
>>>>> 
>>>>> The problem occurs when the value read is converted from string form to a 
>>>>> Ruby timestamp, so it is largely database independent (the exception 
>>>>> being drivers that override the methods, or databases that don't support 
>>>>> timestamps at all).
>>>>> 
>>>>> The underlying problem is the use of to_i to convert from floats to ints 
>>>>> inside the affected methods. As you know, to_i simply truncates the 
>>>>> result and in some cases this causes rounding errors introduced by 
>>>>> inherent inaccuracies in the multiplication operations and decimal 
>>>>> representation to bubble up and affect the least significant digit.
>>>>> 
>>>>> Here's a simple test that illustrates the problem:
>>>>> 
>>>>>      converted = 
>>>>> ActiveRecord::ConnectionAdapters::Column.send('fast_string_to_time', 
>>>>> "2010-01-12 12:34:56.125014")
>>>>>      assert_equal 125014, converted.usec
>>>>> 
>>>>> This test case (and a similar one for #microseconds) fail on plain 
>>>>> vanilla Rails 2.3.5.
>>>>> 
>>>>> I guess the best solution would be to change the ISO_DATETIME regex used 
>>>>> to extract the microsecond-part from timestamps to not include the 
>>>>> decimal point at all and then avoid the to_f and subsequent floating 
>>>>> point multiplication completely inside the failing methods. However, 
>>>>> these regexes are defined as constants on 
>>>>> ActiveRecord::ConnectionAdapters::Column::Format and therefore publicly 
>>>>> available, so the impact of changing these is difficult to ascertain.
>>>>> 
>>>>> A simpler solution is to use round() instead of to_i to convert from the 
>>>>> intermediate floating point result to int. This works (I have verified 
>>>>> that the precision is sufficient for all possible 6-digit cases) but is 
>>>>> about 15% slower than the current method. A small price to pay for 
>>>>> correctness, in my opinion.
>>>>> 
>>>>> I have attached a tiny patch (against 2.3.5) that switches the code to 
>>>>> using round() and a test case that verifies that the method works for a 
>>>>> few problematic cases that fail without the patch.
>>>>> 
>>>>> I have also created a Lighthouse ticket #3693:
>>>>> 
>>>>> https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/3...
>>>>> 
>>>>> Could some of you please take a look and see if the patch is acceptable 
>>>>> and maybe carry it into the code base?
>>>>> 
>>>>> Cheers
>>>>> Jacob
>>>>> 
>>>>> fix_microsecond_conversion.diff
>>>>> 4KViewDownload
>>>>> 
>>>>> 
>>>> -- 
>>>> You received this message because you are subscribed to the Google Groups 
>>>> "Ruby on Rails: Core" group.
>>>> To post to this group, send email to [email protected].
>>>> To unsubscribe from this group, send email to 
>>>> [email protected].
>>>> For more options, visit this group at 
>>>> http://groups.google.com/group/rubyonrails-core?hl=en.
>>>> 
>>>> 
>>> 
>>> -- 
>>> You received this message because you are subscribed to the Google Groups 
>>> "Ruby on Rails: Core" group.
>>> To post to this group, send email to [email protected].
>>> To unsubscribe from this group, send email to 
>>> [email protected].
>>> For more options, visit this group at 
>>> http://groups.google.com/group/rubyonrails-core?hl=en.
>>> 
>>> 
>> 
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To post to this group, send email to [email protected].
>> To unsubscribe from this group, send email to 
>> [email protected].
>> For more options, visit this group at 
>> http://groups.google.com/group/rubyonrails-core?hl=en.
>> 
>> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Ruby on Rails: Core" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to 
> [email protected].
> For more options, visit this group at 
> http://groups.google.com/group/rubyonrails-core?hl=en.
> 
> 

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/rubyonrails-core?hl=en.


Reply via email to