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

Fixes #12011 - Replace .includes(table).where(table) by .eager_load #2768

Closed
wants to merge 1 commit into from

Conversation

dLobatog
Copy link
Member

On Rails 3, .includes(:users).where("users.name = 'daniel'") works because
Rails would be smart enough to figure out the where query uses the includes
table. In this case it'd internally resolve it to eager_load to use the table
users in the query.

However on Rails 4 they decided they didn't want to maintain a parser for SQL,
so we have to append .references(:table) after every
.includes(:table).where("SQL that uses table").
This is not retrocompatible, but we can directly use eager_load on Rails 3 and
4 to avoid using .references.

@dLobatog
Copy link
Member Author

dLobatog commented Oct 1, 2015

Red because Gem::InstallError: fog-google requires Ruby version >= 2.0.

@dLobatog
Copy link
Member Author

dLobatog commented Oct 1, 2015

[test] now that fog-google is pinned

On Rails 3, .includes(:users).where("users.name = 'daniel'") works because
Rails would be smart enough to figure out the where query uses the includes
table. In this case it'd internally resolve it to eager_load to use the table
users in the query.

However on Rails 4 they decided they didn't want to maintain a parser for SQL,
so we have to append .references(:table) after every
 .includes(:table).where("SQL that uses table").
This is not retrocompatible, but we can directly use eager_load on Rails 3 and
4 to avoid using .references.
@dLobatog
Copy link
Member Author

dLobatog commented Oct 2, 2015

[test]

@shlomizadok
Copy link
Member

Tested. Works well

@domcleal
Copy link
Contributor

domcleal commented Oct 9, 2015

Merged as 7fdb705, thanks @elobato.

@domcleal domcleal closed this Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants