Fix bug with nested conditions passed to .where() #9906

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@DanOlson
Contributor

Recusion in PredicateBuilder.build_from_hash() so that passing a nested
hash of conditions to .where will build a valid SQL query.
This allows queries like these:

Post.joins(author: :organization).
where(authors: { organizations: { name: 'Acme' } } )

This would fix issue #9511
#9511

Previously, that same query's WHERE clause would read:

"... WHERE "authors"."organizations" ...

which would break because the 'authors' table doesn't have an 'organizations' column. This change will instantiate an Arel::Table for 'organizations', and the query will be built correctly.

@DanOlson DanOlson Fix bug with nested conditions passed to .where()
Recusion in PredicateBuilder.build_from_hash() so that passing a nested
hash of conditions to .where will build a valid SQL query.
This allows queries like these:

  Post.joins(author: :organization).
    where(authors: { organizations: { name: 'Acme' } } )
f705c41
@carlosantoniodasilva

I believe such query should be build as:

Post.joins(author: :organization).where(organizations: { name: 'Acme' })

So that you don't need to use authors to get through the organizations table in the where. I think that's been done like that on purpose. /cc @jonleighton

@DanOlson
Contributor

Looking around a bit, it seems this has been reported a few times previously, and is causing problems for people who expect the nested syntax to work: #6718 as an example. The popular library CanCan generates nested conditions, so I imagine there are quite a few apps out there that would rely on this being supported, especially considering it used to work. Although I've been able to find a few mentions of this issue, I haven't found an explanation as to why the behavior was changed. Is there a reason not to support both authors: {organizations: {name: 'Acme'}} AND organizations: {name: 'Acme'}?

Other issues I've found relating to this:
ryanb/cancan#646
ryanb/cancan#830
#9511
#6718 (mentioned above)

Cheers!

@jonleighton
Member

This was changed to fix a security issue, therefore it won't be changed back.

See https://groups.google.com/forum/?hl=en&fromgroups=#!topic/rubyonrails-security/l4L0TEVAz1k for more details.

@jonleighton jonleighton closed this Apr 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment