Move the from bind logic to a FromClause class

Contrary to my previous commit message, it wasn't overkill, and led to
much cleaner code.

[Sean Griffin & anthonynavarre]

activerecord/lib/active_record/relation.rb

@@ -5,12 +5,12 @@ module ActiveRecord
   # = Active Record Relation
   class Relation
     MULTI_VALUE_METHODS  = [:includes, :eager_load, :preload, :select, :group,
-                            :order, :joins, :references, :from_bind,
+                            :order, :joins, :references,
                             :extending, :unscope]
 
-    SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering,
+    SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering,
                             :reverse_order, :distinct, :create_with, :uniq]
-    CLAUSE_METHODS = [:where, :having]
+    CLAUSE_METHODS = [:where, :having, :from]
     INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having]
 
     VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS

activerecord/lib/active_record/relation/from_clause.rb

@@ -0,0 +1,32 @@
+module ActiveRecord
+  class Relation
+    class FromClause
+      attr_reader :value, :name
+
+      def initialize(value, name)
+        @value = value
+        @name = name
+      end
+
+      def binds
+        if value.is_a?(Relation)
+          value.arel.bind_values + value.bind_values
+        else
+          []
+        end
+      end
+
+      def merge(other)
+        self
+      end
+
+      def empty?
+        value.nil?
+      end
+
+      def self.empty
+        new(nil, nil)
+      end
+    end
+  end
+end

activerecord/lib/active_record/relation/merger.rb

@@ -51,7 +51,7 @@ def initialize(relation, other)
 
       NORMAL_VALUES = Relation::VALUE_METHODS -
                       Relation::CLAUSE_METHODS -
-                      [:joins, :order, :reverse_order, :lock, :create_with, :reordering, :from, :from_bind] # :nodoc:
+                      [:joins, :order, :reverse_order, :lock, :create_with, :reordering] # :nodoc:
 
       def normal_values
         NORMAL_VALUES
@@ -120,9 +120,7 @@ def merge_multi_values
       end
 
       def merge_single_values
-        relation.from_value        ||= other.from_value
-        relation.from_bind_values    = other.from_bind_values unless relation.from_value
-        relation.lock_value        ||= other.lock_value
+        relation.lock_value ||= other.lock_value
 
         unless other.create_with_value.blank?
           relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value)

activerecord/lib/active_record/relation/query_methods.rb

@@ -1,8 +1,9 @@
-require 'active_support/core_ext/array/wrap'
-require 'active_support/core_ext/string/filters'
-require 'active_model/forbidden_attributes_protection'
+require "active_record/relation/from_clause"
 require "active_record/relation/where_clause"
 require "active_record/relation/where_clause_factory"
+require 'active_model/forbidden_attributes_protection'
+require 'active_support/core_ext/array/wrap'
+require 'active_support/core_ext/string/filters'
 
 module ActiveRecord
   module QueryMethods
@@ -93,7 +94,7 @@ def #{name}_clause=(value)                   # def where_clause=(value)
     end
 
     def bind_values
-      from_bind_values + where_clause.binds + having_clause.binds
+      from_clause.binds + where_clause.binds + having_clause.binds
     end
 
     def create_with_value # :nodoc:
@@ -740,12 +741,7 @@ def from(value, subquery_name = nil)
     end
 
     def from!(value, subquery_name = nil) # :nodoc:
-      self.from_value = [value, subquery_name]
-      if value.is_a? Relation
-        self.from_bind_values = value.arel.bind_values + value.bind_values
-      else
-        self.from_bind_values = []
-      end
+      self.from_clause = Relation::FromClause.new(value, subquery_name)
       self
     end
 
@@ -870,7 +866,7 @@ def build_arel
       build_select(arel, select_values.uniq)
 
       arel.distinct(distinct_value)
-      arel.from(build_from) if from_value
+      arel.from(build_from) unless from_clause.empty?
       arel.lock(lock_value) if lock_value
 
       arel
@@ -932,7 +928,8 @@ def association_for_table(table_name)
     end
 
     def build_from
-      opts, name = from_value
+      opts = from_clause.value
+      name = from_clause.name
       case opts
       when Relation
         name ||= 'subquery'
@@ -1097,5 +1094,9 @@ def where_clause_factory
       @where_clause_factory ||= Relation::WhereClauseFactory.new(klass, predicate_builder)
     end
     alias having_clause_factory where_clause_factory
+
+    def new_from_clause
+      Relation::FromClause.empty
+    end
   end
 end

activerecord/test/cases/relation/mutation_test.rb

@@ -28,7 +28,7 @@ def relation
       @relation ||= Relation.new FakeKlass.new('posts'), Post.arel_table, Post.predicate_builder
     end
 
-    (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select, :from_bind]).each do |method|
+    (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select]).each do |method|
       test "##{method}!" do
         assert relation.public_send("#{method}!", :foo).equal?(relation)
         assert_equal [:foo], relation.public_send("#{method}_values")
@@ -81,7 +81,7 @@ def relation
       assert_equal [], relation.extending_values
     end
 
-    (Relation::SINGLE_VALUE_METHODS - [:from, :lock, :reordering, :reverse_order, :create_with]).each do |method|
+    (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with]).each do |method|
       test "##{method}!" do
         assert relation.public_send("#{method}!", :foo).equal?(relation)
         assert_equal :foo, relation.public_send("#{method}_value")
@@ -90,7 +90,7 @@ def relation
 
     test '#from!' do
       assert relation.from!('foo').equal?(relation)
-      assert_equal ['foo', nil], relation.from_value
+      assert_equal 'foo', relation.from_clause.value
     end
 
     test '#lock!' do