I found out today that Range#include? behaves differently on 1.9 than on 1.8 ( http://rhnh.net/2009/08/03/range-include-in-ruby-1-9 ) In order to deal with various edge cases, instead of just checking whether range.first <= value <= range.last, in ruby 1.9 include? steps through all the values in the range, checking for equality. This is of course a lot slower - in my case, checking a date for inclusion in a 100 year Date range (via a validates_inclusion_of) went from instantaneous to over 100ms. I think this sort of thing is probably pretty common in rails apps and that the edge cases this change was designed to address are extremely unlikely to crop up with validates_inclusion_of - people are almost always going to be validating plain old strings, dates etc. The old include? logic is available as cover? in ruby 1.9. What should (if anything) rails do? Options include - change validates_inclusion_of to use cover? for ranges on ruby 1.9 - add an option to validates_inclusion_of to use cover? instead of include - add a new validation Personally, in the interest of squashing hidden gotchas when migrating to 1.9, I''d be inclined to go for the first of the above options (and maybe let people force the use of include? if they need it for one of these special edge cases). Fred -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Nice find. Yes, I think we should check that :cover? is defined then use it on 1.9. Can you make the patch and create a ticket on Lighthouse? I''ll review the patch and ask someone to get it in. Prem On 16 ก.พ. 2554, at 17:26, Frederick Cheung wrote:> I found out today that Range#include? behaves differently on 1.9 than on 1.8 ( http://rhnh.net/2009/08/03/range-include-in-ruby-1-9 ) > In order to deal with various edge cases, instead of just checking whether range.first <= value <= range.last, in ruby 1.9 include? steps through all the values in the range, checking for equality. > > This is of course a lot slower - in my case, checking a date for inclusion in a 100 year Date range (via a validates_inclusion_of) went from instantaneous to over 100ms. I think this sort of thing is probably pretty common in rails apps and that the edge cases this change was designed to address are extremely unlikely to crop up with validates_inclusion_of - people are almost always going to be validating plain old strings, dates etc. > > The old include? logic is available as cover? in ruby 1.9. What should (if anything) rails do? Options include > - change validates_inclusion_of to use cover? for ranges on ruby 1.9 > - add an option to validates_inclusion_of to use cover? instead of include > - add a new validation > > Personally, in the interest of squashing hidden gotchas when migrating to 1.9, I''d be inclined to go for the first of the above options (and maybe let people force the use of include? if they need it for one of these special edge cases). > > Fred > > -- > 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 rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Cool. I''ll write something up Sent from my iPhone On 16 Feb 2011, at 10:33, Prem Sichanugrist <sikandsak@gmail.com> wrote:> Nice find. Yes, I think we should check that :cover? is defined then use it on 1.9. > > Can you make the patch and create a ticket on Lighthouse? I''ll review the patch and ask someone to get it in. > > Prem > > On 16 ก.พ. 2554, at 17:26, Frederick Cheung wrote: > >> I found out today that Range#include? behaves differently on 1.9 than on 1.8 ( http://rhnh.net/2009/08/03/range-include-in-ruby-1-9 ) >> In order to deal with various edge cases, instead of just checking whether range.first <= value <= range.last, in ruby 1.9 include? steps through all the values in the range, checking for equality. >> >> This is of course a lot slower - in my case, checking a date for inclusion in a 100 year Date range (via a validates_inclusion_of) went from instantaneous to over 100ms. I think this sort of thing is probably pretty common in rails apps and that the edge cases this change was designed to address are extremely unlikely to crop up with validates_inclusion_of - people are almost always going to be validating plain old strings, dates etc. >> >> The old include? logic is available as cover? in ruby 1.9. What should (if anything) rails do? Options include >> - change validates_inclusion_of to use cover? for ranges on ruby 1.9 >> - add an option to validates_inclusion_of to use cover? instead of include >> - add a new validation >> >> Personally, in the interest of squashing hidden gotchas when migrating to 1.9, I''d be inclined to go for the first of the above options (and maybe let people force the use of include? if they need it for one of these special edge cases). >> >> Fred >> >> -- >> 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 rubyonrails-core@googlegroups.com. >> To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. >> 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 rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On Wed, Feb 16, 2011 at 11:26 AM, Frederick Cheung <frederick.cheung@gmail.com> wrote:> I found out today that Range#include? behaves differently on 1.9 than on 1.8 ( http://rhnh.net/2009/08/03/range-include-in-ruby-1-9 ) > In order to deal with various edge cases, instead of just checking whether range.first <= value <= range.last, in ruby 1.9 include? steps through all the values in the range, checking for equality. > > This is of course a lot slowerUgh. And it is not equivalent. Before you could have an infinite number of points in between, now you can''t (and have it working :). For a simple example take ordering in pairs of integers (imagine 2D points in a matrix if you want) defined by (a, b) < (c, d) iff a < c or (a = c, and b < d) define (a, b).next to be (a, b+1). I wrote this off the top of my head, but if I am not mistaken we have now a countable number of pairs between (1, 0) and (2, 0). Namely, (1, 1), (1, 2), .... So, using pseudonotation, ((1, 0)..(2, 0)).include?((3, 0)) won''t even finish in 1.9, while it did before. This is just an example to depict why they are not equivalents, it might be artificial, but I don''t know perhaps there''s some application out there assuming this behavior. Anyway, that''s a good catch, validates_inclusion_of should be based on inequalities. -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 17 Feb 2011, at 08:49, Xavier Noria wrote:> I wrote this off the top of my head, but if I am not mistaken we have > now a countable number of pairs between (1, 0) and (2, 0). Namely, (1, > 1), (1, 2), .... So, using pseudonotation, ((1, 0)..(2, > 0)).include?((3, 0)) won''t even finish in 1.9, while it did before. > > This is just an example to depict why they are not equivalents, it > might be artificial, but I don''t know perhaps there''s some application > out there assuming this behavior. >Doesn''t seem any more artificial than the cases used to justify the 1.9 behaviour. I''ve added patch & ticket at https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6453-use-cover-for-validates_inclusion_of-with-ranges-on-19 . It''s not very beautiful though Fred -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On Fri, Feb 18, 2011 at 7:02 PM, Frederick Cheung <frederick.cheung@gmail.com> wrote:> On 17 Feb 2011, at 08:49, Xavier Noria wrote: >> I wrote this off the top of my head, but if I am not mistaken we have >> now a countable number of pairs between (1, 0) and (2, 0). Namely, (1, >> 1), (1, 2), .... So, using pseudonotation, ((1, 0)..(2, >> 0)).include?((3, 0)) won''t even finish in 1.9, while it did before. >> >> This is just an example to depict why they are not equivalents, it >> might be artificial, but I don''t know perhaps there''s some application >> out there assuming this behavior. >> > Doesn''t seem any more artificial than the cases used to justify the 1.9 behaviour. > > I''ve added patch & ticket at https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6453-use-cover-for-validates_inclusion_of-with-ranges-on-19 . It''s not very beautiful thoughExcellent. I pondered backporting cover? for 1.8 in Active Support (there are some backports, you know) and have one single definition of the validator, but I am not convinced this approach is really justified. Applied, thanks Fred! -- 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 rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.