Skip to content

Commit

Permalink
Fix SELECT COUNT queries when rendering ActiveRecord collections (#…
Browse files Browse the repository at this point in the history
…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]
  • Loading branch information
aar0nr authored and rafaelfranca committed Dec 18, 2020
1 parent e3ab228 commit e4d5f89
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
6 changes: 5 additions & 1 deletion actionview/lib/action_view/renderer/collection_renderer.rb
Expand Up @@ -47,6 +47,10 @@ def each(&blk)
def size
@collection.size
end

def length
@collection.respond_to?(:length) ? @collection.length : size
end
end

class SameCollectionIterator < CollectionIterator # :nodoc:
Expand Down Expand Up @@ -144,7 +148,7 @@ def render_collection(collection, view, path, template, layout, block)
"render_collection.action_view",
identifier: identifier,
layout: layout && layout.virtual_path,
count: collection.size
count: collection.length
) do |payload|
spacer = if @options.key?(:spacer_template)
spacer_template = find_template(@options[:spacer_template], @locals.keys)
Expand Down
28 changes: 28 additions & 0 deletions actionview/test/activerecord/partial_rendering_query_test.rb
@@ -0,0 +1,28 @@
# frozen_string_literal: true

require "active_record_unit"

class PartialRenderingQueryTest < ActiveRecordTestCase
def setup
@view = ActionView::Base
.with_empty_template_cache
.with_view_paths(ActionController::Base.view_paths, {})

@queries = []

@subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload|
@queries << payload[:sql] unless %w[ SCHEMA TRANSACTION ].include?(payload[:name])
end
end

def teardown
ActiveSupport::Notifications.unsubscribe(@subscriber)
end

def test_render_with_relation_collection
@view.render partial: "topics/topic", collection: Topic.all

assert_equal 1, @queries.size
assert_equal 'SELECT "topics".* FROM "topics"', @queries[0]
end
end

0 comments on commit e4d5f89

Please sign in to comment.