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

Rendering ActiveRecord collections as non-derived partials results in SELECT COUNT query #40837

Closed
aar0nr opened this issue Dec 14, 2020 · 3 comments · Fixed by #40870
Closed

Comments

@aar0nr
Copy link
Contributor

aar0nr commented Dec 14, 2020

When a partial is derived from a collection, collection.map is called first which loads the relation.

https://github.com/rails/rails/blob/v6.1.0/actionview/lib/action_view/renderer/collection_renderer.rb#L119

However, when it's non-derived, collection.size gets called first, which results in the SELECT COUNT query.

https://github.com/rails/rails/blob/v6.1.0/actionview/lib/action_view/renderer/collection_renderer.rb#L147

The collection should probably be loaded with a to_a somewhere.

/cc @tenderlove

Steps to reproduce

Render a collection using a non-derived partial:

<%= render collection: Post.all, partial: "post" %>

This results in the following queries:

SELECT COUNT(*) FROM "posts"
CACHE  SELECT COUNT(*) FROM "posts"
Post Load SELECT "posts".* FROM "posts"

Rendering a collection with a derived partial:

<%= render Post.all %>

This results in the following queries:

Post Load SELECT "posts".* FROM "posts"

Expected behavior

Rendering an ActiveRecord collection with a non-derived partial should not execute a SELECT COUNT query.

System configuration

Rails version:

6.1.0

Ruby version:

2.7.1

@aar0nr aar0nr changed the title Rendering ActiveRecord collections as non-derived partials results in unnecessary SELECT COUNT query Rendering ActiveRecord collections as non-derived partials results in SELECT COUNT query Dec 14, 2020
@rafaelfranca
Copy link
Member

That is an interesting issue. Can you investigate where we can load the collection so the behavior is consistent? The only thing you need to keep in mind is that the collection something needs to preload some associations so we need to take that in consideration as well

@aar0nr
Copy link
Contributor Author

aar0nr commented Dec 17, 2020

After a further look, it's a little more complicated. The main issue is that collection.size is being called in two places, but then further along expects the collection to still be an unloaded relation for the PreloadCollectionIterator. In reality, the collection.size information should not available if the collection is a relation, otherwise we have a bunch of phantom SELECT COUNT queries running. I'm not sure how you want to go about fixing it.

@aar0nr
Copy link
Contributor Author

aar0nr commented Dec 17, 2020

Actually, the PreloadCollectionIterator checks if the relation has been loaded, so I think we can just preload to array and count the size. I'll open pull request.

rafaelfranca pushed a commit that referenced this issue Dec 18, 2020
…40870)

* Fix `SELECT COUNT` queries when rendering ActiveRecord collections

Fixes #40837

When rendering collections, calling `size` when the collection is an
ActiveRecord relation causes unwanted `SELECT COUNT(*)` queries. This
change ensures the collection is an array before getting the size, and
also loads the relation for any further array inspections.

* Test queries when rendering relation collections

* Add `length` support to partial collection iterator

Allows getting the size of a relation without duplicating records, but
still loads the relation. The length method existence needs to be
checked because you can pass in an `Enumerator`, which does not respond
to `length`.

* Ensure unsubscribed from notifications after tests

[Rafael Mendonça França + aar0nr]
rafaelfranca pushed a commit that referenced this issue Dec 18, 2020
…40870)

* Fix `SELECT COUNT` queries when rendering ActiveRecord collections

Fixes #40837

When rendering collections, calling `size` when the collection is an
ActiveRecord relation causes unwanted `SELECT COUNT(*)` queries. This
change ensures the collection is an array before getting the size, and
also loads the relation for any further array inspections.

* Test queries when rendering relation collections

* Add `length` support to partial collection iterator

Allows getting the size of a relation without duplicating records, but
still loads the relation. The length method existence needs to be
checked because you can pass in an `Enumerator`, which does not respond
to `length`.

* Ensure unsubscribed from notifications after tests

[Rafael Mendonça França + aar0nr]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants