From a5fcdae0a04d97dbd8fd05af6f352d3e09b2815f Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@thoughtbot.com>
Date: Tue, 27 Jan 2015 09:41:25 -0700
Subject: [PATCH] Move where grouping into `WhereClause`

---
 .../active_record/relation/query_methods.rb   | 12 +------
 .../active_record/relation/where_clause.rb    | 25 ++++++++++++++
 .../test/cases/relation/where_clause_test.rb  | 33 +++++++++++++++++++
 3 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 5df1269890..c8b74490c4 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -852,7 +852,7 @@ module ActiveRecord
 
       build_joins(arel, joins_values.flatten) unless joins_values.empty?
 
-      collapse_wheres(arel, (where_clause.predicates - [''])) #TODO: Add uniq with real value comparison / ignore uniqs that have binds
+      arel.where(where_clause.ast) unless where_clause.empty?
 
       arel.having(*having_clause.predicates.uniq.reject(&:blank?)) if having_clause.any?
 
@@ -911,16 +911,6 @@ module ActiveRecord
       end
     end
 
-    def collapse_wheres(arel, wheres)
-      predicates = wheres.map do |where|
-        next where if ::Arel::Nodes::Equality === where
-        where = Arel.sql(where) if String === where
-        Arel::Nodes::Grouping.new(where)
-      end
-
-      arel.where(Arel::Nodes::And.new(predicates)) if predicates.present?
-    end
-
     def association_for_table(table_name)
       table_name = table_name.to_s
       @klass._reflect_on_association(table_name) ||
diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb
index 8b9ba3e633..6c62332609 100644
--- a/activerecord/lib/active_record/relation/where_clause.rb
+++ b/activerecord/lib/active_record/relation/where_clause.rb
@@ -53,6 +53,10 @@ module ActiveRecord
         }.to_h
       end
 
+      def ast
+        Arel::Nodes::And.new(predicates_with_wrapped_sql_literals)
+      end
+
       def ==(other)
         other.is_a?(WhereClause) &&
           predicates == other.predicates &&
@@ -128,6 +132,27 @@ module ActiveRecord
           columns.include?(column.name)
         end
       end
+
+      def predicates_with_wrapped_sql_literals
+        non_empty_predicates.map do |node|
+          if Arel::Nodes::Equality === node
+            node
+          else
+            wrap_sql_literal(node)
+          end
+        end
+      end
+
+      def non_empty_predicates
+        predicates - ['']
+      end
+
+      def wrap_sql_literal(node)
+        if ::String === node
+          node = Arel.sql(node)
+        end
+        Arel::Nodes::Grouping.new(node)
+      end
     end
   end
 end
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index 6864be2608..511d595f71 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -111,6 +111,39 @@ class ActiveRecord::Relation
       assert_equal expected, where_clause.except("id", "name")
     end
 
+    test "ast groups its predicates with AND" do
+      where_clause = WhereClause.new([
+        table["id"].in([1, 2, 3]),
+        table["name"].eq(bind_param),
+      ], [])
+      expected = Arel::Nodes::And.new(where_clause.predicates)
+
+      assert_equal expected, where_clause.ast
+    end
+
+    test "ast wraps any SQL literals in parenthesis" do
+      random_object = Object.new
+      where_clause = WhereClause.new([
+        table["id"].in([1, 2, 3]),
+        "foo = bar",
+        random_object,
+      ], [])
+      expected = Arel::Nodes::And.new([
+        table["id"].in([1, 2, 3]),
+        Arel::Nodes::Grouping.new(Arel.sql("foo = bar")),
+        Arel::Nodes::Grouping.new(random_object),
+      ])
+
+      assert_equal expected, where_clause.ast
+    end
+
+    test "ast removes any empty strings" do
+      where_clause = WhereClause.new([table["id"].in([1, 2, 3])], [])
+      where_clause_with_empty = WhereClause.new([table["id"].in([1, 2, 3]), ''], [])
+
+      assert_equal where_clause.ast, where_clause_with_empty.ast
+    end
+
     private
 
     def table