search

rails / arel

A Relational Algebra
2.06k stars 390 forks source link

Allowing between to use more flexible unboudned ranges #457

Closed arjes closed 7 years ago

arjes commented 7 years ago

This PR provides a more flexible implementation of the unbounded range between comparison.

The specific need that drove this change was the ability to compare unbounded date ranges, using rails this meant where(created_at: Time.now...DateTime::Infinity.new)

I've duplicated all the Float::INFINITY specs with a DateTime::Infinity, however the -DateTime::Infinity tests are only valid for -DateTime::Infinity.new..DateTime::Infinity.new This is because -DateTime::Infinity.new..DateTime.now throws "ArgumentError: bad value for range". I've identified the issues causing this to be, what appears to be, and incomplete implementation of <=> in DateTime::Infinity. I've monkey patched this locally and will see about an upstream change, then add tests for it here.

rails-bot commented 7 years ago

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @sgrif (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

sgrif commented 7 years ago

Thanks, but the plan is actually to remove this behavior from arel in the future and have it handled elsewhere.

arjes commented 7 years ago

Makes sense to not have arel mucking with the queries it was instructed to create. I assume this will be pushed up into rails then for their implementation. Thanks for your time.