From b64b7545106db0744c054ef615a952fa1e962e60 Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Tue, 12 Jan 2016 15:47:05 -0700
Subject: [PATCH] Revert "Change `WhereClause#merge` to same named columns on
 diff tables"

This reverts commit 5d41cb3bfd6b19833261622ce5d339b1e580bd8b.

This implementation does not properly handle cases involving predicates
which are not associated with a bind param. I have the fix in mind, but
don't have time to implement just yet. It will be more similar to #22823
than not.
---
 .../active_record/relation/where_clause.rb    | 21 ++++++++++---------
 .../test/cases/relation/where_clause_test.rb  | 17 ++-------------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb
index e39dbac0d5..2c2d6cfa47 100644
--- a/activerecord/lib/active_record/relation/where_clause.rb
+++ b/activerecord/lib/active_record/relation/where_clause.rb
@@ -18,10 +18,9 @@ def +(other)
       end
 
       def merge(other)
-        conflict_indices = indices_of_predicates_referenced_by(other).to_set
         WhereClause.new(
-          non_conflicting(predicates, conflict_indices) + other.predicates,
-          non_conflicting(binds, conflict_indices) + other.binds,
+          predicates_unreferenced_by(other) + other.predicates,
+          non_conflicting_binds(other) + other.binds,
         )
       end
 
@@ -98,20 +97,22 @@ def referenced_columns
 
       private
 
-      def indices_of_predicates_referenced_by(other)
-        predicates.each_with_index.select do |(n, _)|
+      def predicates_unreferenced_by(other)
+        predicates.reject do |n|
           equality_node?(n) && other.referenced_columns.include?(n.left)
-        end.map(&:last)
-      end
-
-      def non_conflicting(values, conflict_indices)
-        values.reject.with_index { |_, i| conflict_indices.include?(i) }
+        end
       end
 
       def equality_node?(node)
         node.respond_to?(:operator) && node.operator == :==
       end
 
+      def non_conflicting_binds(other)
+        conflicts = referenced_columns & other.referenced_columns
+        conflicts.map! { |node| node.name.to_s }
+        binds.reject { |attr| conflicts.include?(attr.name) }
+      end
+
       def inverted_predicates
         predicates.map { |node| invert_predicate(node) }
       end
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index c3296b28fd..c20ed94d90 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -62,21 +62,8 @@ class WhereClauseTest < ActiveRecord::TestCase
     end
 
     test "merge allows for columns with the same name from different tables" do
-      table2 = Arel::Table.new("table2")
-      a = WhereClause.new(
-        [table["id"].eq(bind_param), table2["id"].eq(bind_param), table["name"].eq(bind_param)],
-        [attribute("id", 3), attribute("id", 2), attribute("name", "Jim")]
-      )
-      b = WhereClause.new(
-        [table["id"].eq(bind_param), table["name"].eq(bind_param)],
-        [attribute("id", 1), attribute("name", "Sean")],
-      )
-      expected = WhereClause.new(
-        [table2["id"].eq(bind_param), table["id"].eq(bind_param), table["name"].eq(bind_param)],
-        [attribute("id", 2), attribute("id", 1), attribute("name", "Sean")],
-      )
-
-      assert_equal expected, a.merge(b)
+      skip "This is not possible as of 4.2, and the binds do not yet contain sufficient information for this to happen"
+      # We might be able to change the implementation to remove conflicts by index, rather than column name
     end
 
     test "a clause knows if it is empty" do
-- 
GitLab