From 1f98493e4a3c53e53666ff740860b20dcd6d08ec Mon Sep 17 00:00:00 2001 From: Andrey Deryabin Date: Fri, 27 Apr 2012 10:15:28 +0700 Subject: handle joins/includes correctly for pluck and calculation. Fix #5990 --- activerecord/lib/active_record/relation/calculations.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 54c93332bb..d457062341 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 eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) construct_relation_for_association_calculations.calculate(operation, column_name, options) else perform_calculation(operation, column_name, options) @@ -155,6 +156,10 @@ module ActiveRecord column_name = "#{table_name}.#{column_name}" end + if eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) + return construct_relation_for_association_calculations.pluck(column_name) + end + result = klass.connection.select_all(select(column_name).arel, nil, bind_values) key = result.columns.first -- cgit v1.2.3 From 60ffe41980f3989637bdf537341288288aa9cc39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 19 Jun 2012 00:36:30 -0300 Subject: Extract conditional to a method to avoid duplication Also use if/else block to not use short circuit return --- .../lib/active_record/relation/calculations.rb | 34 ++++++++++++---------- 1 file changed, 19 insertions(+), 15 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index d457062341..22c3e6a324 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -108,7 +108,7 @@ module ActiveRecord if relation.equal?(self) - if eager_loading? || (includes_values.present? && (column_name || 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) @@ -156,25 +156,25 @@ module ActiveRecord column_name = "#{table_name}.#{column_name}" end - if eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) - return construct_relation_for_association_calculations.pluck(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 @@ -190,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 -- cgit v1.2.3 From 1579f128ab28e36bbf8e91872857472e7c705720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 19 Jun 2012 20:49:27 -0300 Subject: Remove unneeded code since pluck is respecting joins now --- .../active_record/associations/collection_association.rb | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) (limited to 'activerecord/lib') 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 -- cgit v1.2.3