From 0aae7806c10e3e3cce671c23dac6a8958ba89629 Mon Sep 17 00:00:00 2001 From: Maxime Lapointe Date: Wed, 10 Aug 2016 17:27:32 -0400 Subject: Fix count which would sometimes force a DISTINCT The current behaviour of checking if there is a LEFT OUTER JOIN arel node to detect if we are doing eager_loading is wrong. This problem wasn't frequent before as only some pretty specific cases would add a LEFT OUTER JOIN arel node. However, the recent new feature left_outer_joins also add this node and made this problem happen frequently. Since in the perform_calculation function, we don't have access to eager_loading information, I had to extract the logic for the distinct out to the calculate method. As I was in the file for left_outer_join tests, I fixed a few that had bugs and I replaced some that were really weak with something that will catch more issues. In relation tests, the first test I changed would have failed if it had validated the hash returned by count instead of just checking how many pairs were in it. This is because this merge of join currently transforms the join node into an outer join node, which then made count do a distinct. So before this change, the return was {1=>1, 4=>1, 5=>1}. --- activerecord/lib/active_record/relation/calculations.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index ecf3700aab..a4962879ab 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -117,7 +117,10 @@ module ActiveRecord end if has_include?(column_name) - construct_relation_for_association_calculations.calculate(operation, column_name) + relation = construct_relation_for_association_calculations + relation = relation.distinct if operation.to_s.downcase == "count" + + relation.calculate(operation, column_name) else perform_calculation(operation, column_name) end @@ -198,11 +201,6 @@ module ActiveRecord if operation == "count" column_name ||= select_for_count - - unless arel.ast.grep(Arel::Nodes::OuterJoin).empty? - distinct = true - end - column_name = primary_key if column_name == :all && distinct distinct = nil if column_name =~ /\s*DISTINCT[\s(]+/i end -- cgit v1.2.3