Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

where ignores keys if values are empty hashes #6960

Closed
giorgian opened this issue Jul 4, 2012 · 3 comments
Closed

where ignores keys if values are empty hashes #6960

giorgian opened this issue Jul 4, 2012 · 3 comments

Comments

@giorgian
Copy link

giorgian commented Jul 4, 2012

In version 3.2.6:

User.where(:token => {}).first

    User Load (0.3ms)  SELECT "users".* FROM "users" LIMIT 1

This may lead to trouble if I check for, say, params[:token] to exist but not for it to be nonblank.

@mauricio
Copy link
Contributor

mauricio commented Jul 4, 2012

I don't get it. What would you want it to do?

@giorgian
Copy link
Author

giorgian commented Jul 5, 2012

Actually, I have two different issues with the way this currently works.

My first concern is that, as a RoR user, I tend to trust it very much and to believe that it'll protect me when it comes to injections and other nasty things; for that reason, I'd like AR never to accept an hash coming from params, unless explicitly told to. That, though, is out of the scope of this issue.
Users are often encouraged to do things like Something.where(:id => params[:id]).first. What I'd expect, if a bad param is passed, is for this query to return nothing, not to return what the the first object in the table happens to be.

So, returning an empty relation seems to me the right thing to do in this case.

My other concern is about semantics: the expression :some_column => {} is interpreted, as I understand it, roughly as: do nothing for the table some_column. When I see it, though, I'm naively led to interpret it as: 'some_column = ?', {}, which evaluates to :some_column => nil.

So, interpreting it as :some_column => nil is another possibility; it's also what happens for :some_column => [].

Any of these two alternatives would, IMO, be preferable to the current behavior.

@dmathieu
Copy link
Contributor

dmathieu commented Jul 5, 2012

I pushed patch about this in #6971

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants