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}.

1 files changed, 17 insertions, 8 deletions

diff --git a/activerecord/test/cases/associations/left_outer_join_association_test.rb b/activerecord/test/cases/associations/left_outer_join_association_test.rb
index d814229f64..2cc6468827 100644
--- a/activerecord/test/cases/associations/left_outer_join_association_test.rb
+++ b/activerecord/test/cases/associations/left_outer_join_association_test.rb
@@ -25,23 +25,32 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
     end
   end
 
-  def test_construct_finder_sql_executes_a_left_outer_join
-    assert_not_equal Author.count, Author.joins(:posts).count
-    assert_equal Author.count, Author.left_outer_joins(:posts).count
+  def test_left_outer_joins_count_is_same_as_size_of_loaded_results
+    assert_equal 17, Post.left_outer_joins(:comments).to_a.size
+    assert_equal 17, Post.left_outer_joins(:comments).count
   end
 
-  def test_left_outer_join_by_left_joins
-    assert_not_equal Author.count, Author.joins(:posts).count
-    assert_equal Author.count, Author.left_joins(:posts).count
+  def test_left_joins_aliases_left_outer_joins
+    assert_equal Post.left_outer_joins(:comments).to_sql, Post.left_joins(:comments).to_sql
+  end
+
+  def test_left_outer_joins_return_has_value_for_every_comment
+    all_post_ids = Post.pluck(:id)
+    assert_equal all_post_ids, all_post_ids & Post.left_outer_joins(:comments).pluck(:id)
+  end
+
+  def test_left_outer_joins_actually_does_a_left_outer_join
+    queries = capture_sql { Author.left_outer_joins(:posts).to_a }
+    assert queries.any? { |sql| /LEFT OUTER JOIN/i.match?(sql) }
   end
 
   def test_construct_finder_sql_ignores_empty_left_outer_joins_hash
-    queries = capture_sql { Author.left_outer_joins({}) }
+    queries = capture_sql { Author.left_outer_joins({}).to_a }
     assert queries.none? { |sql| /LEFT OUTER JOIN/i.match?(sql) }
   end
 
   def test_construct_finder_sql_ignores_empty_left_outer_joins_array
-    queries = capture_sql { Author.left_outer_joins([]) }
+    queries = capture_sql { Author.left_outer_joins([]).to_a }
     assert queries.none? { |sql| /LEFT OUTER JOIN/i.match?(sql) }
   end