diff options
3 files changed, 67 insertions, 23 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index e94fe35170..2447914640 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -50,18 +50,7 @@ module ActiveRecord end else column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}" - relation = scoped - - including = (relation.eager_load_values + relation.includes_values).uniq - - if including.any? - join_dependency = ActiveRecord::Associations::JoinDependency.new(reflection.klass, including, []) - relation = join_dependency.join_associations.inject(relation) do |r, association| - association.join_relation(r) - end - end - - relation.pluck(column) + scoped.pluck(column) end end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 54c93332bb..22c3e6a324 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -107,7 +107,8 @@ module ActiveRecord relation = with_default_scope if relation.equal?(self) - if eager_loading? || (includes_values.present? && references_eager_loaded_tables?) + + if has_include?(column_name) construct_relation_for_association_calculations.calculate(operation, column_name, options) else perform_calculation(operation, column_name, options) @@ -155,21 +156,25 @@ module ActiveRecord column_name = "#{table_name}.#{column_name}" end - result = klass.connection.select_all(select(column_name).arel, nil, bind_values) + if has_include?(column_name) + construct_relation_for_association_calculations.pluck(column_name) + else + result = klass.connection.select_all(select(column_name).arel, nil, bind_values) - key = result.columns.first - column = klass.column_types.fetch(key) { - result.column_types.fetch(key) { - Class.new { def type_cast(v); v; end }.new + key = result.columns.first + column = klass.column_types.fetch(key) { + result.column_types.fetch(key) { + Class.new { def type_cast(v); v; end }.new + } } - } - result.map do |attributes| - raise ArgumentError, "Pluck expects to select just one attribute: #{attributes.inspect}" unless attributes.one? + result.map do |attributes| + raise ArgumentError, "Pluck expects to select just one attribute: #{attributes.inspect}" unless attributes.one? - value = klass.initialize_attributes(attributes).values.first + value = klass.initialize_attributes(attributes).values.first - column.type_cast(value) + column.type_cast(value) + end end end @@ -185,6 +190,10 @@ module ActiveRecord private + def has_include?(column_name) + eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) + end + def perform_calculation(operation, column_name, options = {}) operation = operation.to_s.downcase diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index a279b0e77c..f748b897ee 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -420,6 +420,40 @@ class CalculationsTest < ActiveRecord::TestCase Account.where("credit_limit > 50").from('accounts').maximum(:credit_limit) end + def test_maximum_with_not_auto_table_name_prefix_if_column_included + Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) + + # TODO: Investigate why PG isn't being typecast + if current_adapter?(:PostgreSQLAdapter) + assert_equal "7", Company.includes(:contracts).maximum(:developer_id) + else + assert_equal 7, Company.includes(:contracts).maximum(:developer_id) + end + end + + def test_minimum_with_not_auto_table_name_prefix_if_column_included + Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) + + # TODO: Investigate why PG isn't being typecast + if current_adapter?(:PostgreSQLAdapter) + assert_equal "7", Company.includes(:contracts).minimum(:developer_id) + else + assert_equal 7, Company.includes(:contracts).minimum(:developer_id) + end + end + + def test_sum_with_not_auto_table_name_prefix_if_column_included + Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) + + # TODO: Investigate why PG isn't being typecast + if current_adapter?(:MysqlAdapter) || current_adapter?(:PostgreSQLAdapter) + assert_equal "7", Company.includes(:contracts).sum(:developer_id) + else + assert_equal 7, Company.includes(:contracts).sum(:developer_id) + end + end + + def test_from_option_with_specified_index if Edge.connection.adapter_name == 'MySQL' or Edge.connection.adapter_name == 'Mysql2' assert_equal Edge.count(:all), Edge.from('edges USE INDEX(unique_edge_index)').count(:all) @@ -477,6 +511,11 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [c.id], Company.joins(:contracts).pluck(:id) end + def test_pluck_if_table_included + c = Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) + assert_equal [c.id], Company.includes(:contracts).where("contracts.id" => c.contracts.first).pluck(:id) + end + def test_pluck_not_auto_table_name_prefix_if_column_joined Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) assert_equal [7], Company.joins(:contracts).pluck(:developer_id) @@ -500,4 +539,11 @@ class CalculationsTest < ActiveRecord::TestCase def test_plucks_with_ids assert_equal Company.all.map(&:id).sort, Company.ids.sort end + + def test_pluck_not_auto_table_name_prefix_if_column_included + Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) + ids = Company.includes(:contracts).pluck(:developer_id) + assert_equal Company.count, ids.length + assert_equal [7], ids.compact + end end |