I found out today that Range#include? behaves differently on 1.9 than on 1.8 ( 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).
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.
Cool. I'll write something up
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
Doesn't seem any more artificial than the cases used to justify the 1.9 behaviour.
I've added patch & ticket at #6453 Use cover? for validates_inclusion_of with Ranges on 1.9 - Ruby on Rails - rails . It's not very beautiful though
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!