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

Don't eager load when plucking with includes/limit #36273

Closed
wants to merge 1 commit into from

Conversation

dvandersluis
Copy link
Contributor

Summary

Consider a Company c that has many Contracts, and the following statement:

Company.where(id: c.id).includes(:contracts).limit(1).pluck('contracts.id')

The pluck runs two queries, one to get distinct values of the main record primary key, which has the LIMIT applied, and a second to get the pluck values, with is filtered by the IDs retrieved in the first query:

SELECT DISTINCT "companies"."id" FROM "companies" LEFT OUTER JOIN "contracts" ON "contracts"."company_id" = "companies"."id" WHERE "companies"."id" = ? LIMIT ?  [["id", 12], ["LIMIT", 1]]
SELECT "contracts"."developer_id" FROM "companies" LEFT OUTER JOIN "contracts" ON "contracts"."company_id" = "companies"."id" WHERE "companies"."id" = ? AND "companies"."id" = ?  [["id", 12], ["id", 12]]

The limit is applied only to the number of records retrieved only from the first query, which means that in the pluck above, multiple results are returned, despite asking for 1.

As well, running a second query has a performance impact, especially if the query isn't hitting an appropriate index (as I found in my production app). In the best case scenario, it's still a second query for no benefit. Since a pluck statement isn't going to instantiate any AR objects, nothing will be preloaded anyways.

This change makes includes.limit.pluck behave identically to left_joins.limit.pluck (same queries, same results).

Fixes #36217.

c = Company.create!(name: "test", contracts: [Contract.new(developer_id: 7), Contract.new(developer_id: 8)])
assert_equal Company.where(id: c.id).includes(:contracts).limit(1).pluck(:'contracts.developer_id'),
Company.where(id: c.id).left_outer_joins(:contracts).limit(1).pluck(:'contracts.developer_id')
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this test is needed or useful, so I'm happy to remove it if desired.

@rails-bot
Copy link

rails-bot bot commented Dec 17, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 17, 2019
@rails-bot rails-bot bot closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

includes(has-many).limit.pluck behaves differently than left_joins(has-many).limit.pluck
1 participant