rails-style-guide icon indicating copy to clipboard operation
rails-style-guide copied to clipboard

Where with Ranges: "good" vs "bad" are not equivalent

Open justindell opened this issue 3 years ago • 3 comments
trafficstars

Using an infinite range actually results in an inclusive query most of the time. Mirroring the documentation (using a "Client" model since my example app doesn't have "User")

Client.where("created_at > ?", 7.days.ago).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE (created_at > '2022-06-24 11:00:58.795520')"
Client.where(created_at: 7.days.ago..).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE \"clients\".\"created_at\" >= '2022-06-24 11:00:50.103960'"

Notice the second query uses >= instead of >. Curiously, rails doesn't seem to be consistent between .. and ...

Client.where(created_at: 7.days.ago..).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE \"clients\".\"created_at\" >= '2022-06-24 11:06:22.127959'"
Client.where(created_at: 7.days.ago...).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE \"clients\".\"created_at\" >= '2022-06-24 11:06:25.876704'"
Client.where(created_at: ..7.days.ago).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE \"clients\".\"created_at\" <= '2022-06-24 11:06:32.801216'"
Client.where(created_at: ...7.days.ago).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE \"clients\".\"created_at\" < '2022-06-24 11:06:35.632238'"

I don't know if this is ActiveRecord's intended behavior, but I avoid using infinite ranges because of this inconsistency.

justindell avatar Jul 01 '22 16:07 justindell

Thanks for reporting!

(...2).exclude_end? # => true
(..2).exclude_end? # => false
(2...).exclude_end? # => true
(2..).exclude_end? # => false

Rails makes no difference between 7.days.ago.. and 7.days.ago..., and it seems to be a bug. Do you have the same impression?

Since we're recommending this as a replacement in our examples:

# bad
User.where("created_at > ?", 7.days.ago)

# good
User.where(created_at: 7.days.ago..)

and it's actually incorrect, would you like to send a PR with correct examples and maybe more details?

pirj avatar Jul 01 '22 19:07 pirj

It looks like this is intended behavior, because ranges are inclusive or exclusive based on their ending and beginnings are always inclusive.

There's some discussion here: https://github.com/rails/rails/pull/40628

You can get the intended behavior by using where.not, but I find that to be confusing and counter-intuitive

 Client.where.not(id: ..7).to_sql
# "SELECT \"clients\".* FROM \"clients\" WHERE \"clients\".\"id\" > 7"

I'll work on a PR to make this behavior clearer, but personally I think I'll stick with ranges only with defined start's and end's. With endless ranges, I think the comparative conditions format is more clear and less prone to unexpected bugs.

justindell avatar Jul 02 '22 14:07 justindell

You're right! I've completely missed that there is no difference between an inclusive and exclusive endless ranges, TIL. Thanks for the interesting reference.

pirj avatar Jul 03 '22 09:07 pirj