Change WhereClause#merge to same named columns on diff tables

While the predicates are an arel equality node where the left side is a
full arel attribute, the binds just have the name of the column and
nothing else. This means that while splitting the predicates can include
the table as a factor, the binds cannot. It's entirely possible that we
might be able to have the bind params carry a bit more information (I
don't believe the name is used for anything but logging), and that is
probably a worthwhile change to make in the future.

However the simplest (and likely slightly faster) solution is to simply
use the indices of the conflicts in both cases. This means that we only
have to compute the collision space once, instead of twice even though
we're doing an additional array iteration. Regardless, this method isn't
a performance hotspot.

Close #22823.

[Ben Woosley & Sean Griffin]

activerecord/lib/active_record/relation/where_clause.rb

@@ -18,9 +18,10 @@ module ActiveRecord
       end
 
       def merge(other)
+        conflict_indices = indices_of_predicates_referenced_by(other).to_set
         WhereClause.new(
-          predicates_unreferenced_by(other) + other.predicates,
-          non_conflicting_binds(other) + other.binds,
+          non_conflicting(predicates, conflict_indices) + other.predicates,
+          non_conflicting(binds, conflict_indices) + other.binds,
         )
       end
 
@@ -97,22 +98,20 @@ module ActiveRecord
 
       private
 
-      def predicates_unreferenced_by(other)
-        predicates.reject do |n|
+      def indices_of_predicates_referenced_by(other)
+        predicates.each_with_index.select 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
 
       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

activerecord/test/cases/relation/where_clause_test.rb

@@ -62,8 +62,21 @@ class ActiveRecord::Relation
     end
 
     test "merge allows for columns with the same name from different tables" do
-      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
+      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)
     end
 
     test "a clause knows if it is empty" do