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

pluck should use records (load_target) when loaded? is true #25976

Merged
merged 1 commit into from
Aug 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion activerecord/lib/active_record/associations/collection_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ module Associations
# is computed directly through SQL and does not trigger by itself the
# instantiation of the actual post records.
class CollectionProxy < Relation
delegate(*(ActiveRecord::Calculations.public_instance_methods - [:count]), to: :scope)
delegate :exists?, :update_all, :arel, to: :scope

def initialize(klass, association) #:nodoc:
Expand Down Expand Up @@ -738,6 +737,14 @@ def count(column_name = nil, &block)
@association.count(column_name, &block)
end

def calculate(operation, column_name)
null_scope? ? scope.calculate(operation, column_name) : super
end

def pluck(*column_names)
null_scope? ? scope.pluck(*column_names) : super
end

# Returns the size of the collection. If the collection hasn't been loaded,
# it executes a <tt>SELECT COUNT(*)</tt> query. Else it calls <tt>collection.size</tt>.
#
Expand Down Expand Up @@ -1073,6 +1080,10 @@ def reset

private

def null_scope?
@association.null_scope?
end

def exec_queries
load_target
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation/calculations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def calculate(operation, column_name)
#
def pluck(*column_names)
if loaded? && (column_names.map(&:to_s) - @klass.attribute_names - @klass.attribute_aliases.keys).empty?
return @records.pluck(*column_names)
return records.pluck(*column_names)
end

if has_include?(column_names.first)
Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/cases/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,13 @@ def test_scoped_allows_conditions
assert_no_queries { david.posts.first! }
end

def test_pluck_uses_loaded_target
david = authors(:david)
assert_equal david.posts.pluck(:title), david.posts.load.pluck(:title)
assert david.posts.loaded?
assert_no_queries { david.posts.pluck(:title) }
end

def test_reset_unloads_target
david = authors(:david)
david.posts.reload
Expand Down