Use immutable relation objects to generate queries.

6 files changed, 87 insertions, 92 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index d0322280d9..c0741bb572 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1682,14 +1682,14 @@ module ActiveRecord
             relation = association.join_relation(relation)
           end
 
-          relation.joins!(construct_join(options[:joins], scope)).
-            select!(column_aliases(join_dependency)).
-            group!(construct_group(options[:group], options[:having], scope)).
-            order!(construct_order(options[:order], scope)).
-            conditions!(construct_conditions(options[:conditions], scope))
+          relation = relation.joins(construct_join(options[:joins], scope)).
+            select(column_aliases(join_dependency)).
+            group(construct_group(options[:group], options[:having], scope)).
+            order(construct_order(options[:order], scope)).
+            conditions(construct_conditions(options[:conditions], scope))
 
-          relation.conditions!(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
-          relation.limit!(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections)
+          relation = relation.conditions(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
+          relation = relation.limit(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections)
 
           relation
         end
@@ -1725,14 +1725,13 @@ module ActiveRecord
             relation = association.join_relation(relation)
           end
 
-          relation.joins!(construct_join(options[:joins], scope)).
-            conditions!(construct_conditions(options[:conditions], scope)).
-            group!(construct_group(options[:group], options[:having], scope)).
-            order!(construct_order(options[:order], scope)).
-            limit!(construct_limit(options[:limit], scope)).
-            offset!(construct_limit(options[:offset], scope))
-
-          relation.select!(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", construct_order(options[:order], scope(:find)).join(",")))
+          relation = relation.joins(construct_join(options[:joins], scope)).
+            conditions(construct_conditions(options[:conditions], scope)).
+            group(construct_group(options[:group], options[:having], scope)).
+            order(construct_order(options[:order], scope)).
+            limit(construct_limit(options[:limit], scope)).
+            offset(construct_limit(options[:offset], scope)).
+            select(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", construct_order(options[:order], scope(:find)).join(",")))
 
           sanitize_sql(relation.to_sql)
         end
@@ -2222,12 +2221,12 @@ module ActiveRecord
 
             def join_relation(joining_relation, join = nil)
               if relation.is_a?(Array)
-                joining_relation.joins!(relation.first, join_type).on!(association_join.first)
-                joining_relation.joins!(relation.last, join_type).on!(association_join.last)
+                joining_relation.
+                  joins(relation.first, join_type).on(association_join.first).
+                  joins(relation.last, join_type).on(association_join.last)
               else
-                joining_relation.joins!(relation, join_type).on!(association_join)
+                joining_relation.joins(relation, join_type).on(association_join)
               end
-              joining_relation
             end
 
             protected
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 2b79788b89..b23ce53d43 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -869,15 +869,15 @@ module ActiveRecord #:nodoc:
         relation = arel_table
 
         if conditions = construct_conditions(conditions, scope)
-          relation.conditions!(Arel::SqlLiteral.new(conditions))
+          relation = relation.conditions(Arel::SqlLiteral.new(conditions))
         end
 
-        if options.has_key?(:limit) || (scope && scope[:limit])
+        relation = if options.has_key?(:limit) || (scope && scope[:limit])
           # Only take order from scope if limit is also provided by scope, this
           # is useful for updating a has_many association with a limit.
-          relation.order!(construct_order(options[:order], scope)).limit!(construct_limit(options[:limit], scope))
+          relation.order(construct_order(options[:order], scope)).limit(construct_limit(options[:limit], scope))
         else
-          relation.order!(options[:order])
+          relation.order(options[:order])
         end
 
         relation.update(sanitize_sql_for_assignment(updates))
@@ -932,7 +932,7 @@ module ActiveRecord #:nodoc:
       # associations or call your <tt>before_*</tt> or +after_destroy+ callbacks, use the +destroy_all+ method instead.
       def delete_all(conditions = nil)
         if conditions
-          arel_table.conditions!(Arel::SqlLiteral.new(construct_conditions(conditions, scope(:find)))).delete
+          arel_table.conditions(Arel::SqlLiteral.new(construct_conditions(conditions, scope(:find)))).delete
         else
           arel_table.delete
         end
@@ -1535,7 +1535,7 @@ module ActiveRecord #:nodoc:
 
       def arel_table(table = nil)
         table = table_name if table.blank?
-        Relation.new(self, table)
+        Relation.new(self, Arel::Table.new(table))
       end
 
       private
@@ -1720,13 +1720,13 @@ module ActiveRecord #:nodoc:
         def construct_finder_arel(options = {}, scope = scope(:find))
           # TODO add lock to Arel
           relation = arel_table(options[:from]).
-            joins!(construct_join(options[:joins], scope)).
-            conditions!(construct_conditions(options[:conditions], scope)).
-            select!(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))).
-            group!(construct_group(options[:group], options[:having], scope)).
-            order!(construct_order(options[:order], scope)).
-            limit!(construct_limit(options[:limit], scope)).
-            offset!(construct_offset(options[:offset], scope))
+            joins(construct_join(options[:joins], scope)).
+            conditions(construct_conditions(options[:conditions], scope)).
+            select(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))).
+            group(construct_group(options[:group], options[:having], scope)).
+            order(construct_order(options[:order], scope)).
+            limit(construct_limit(options[:limit], scope)).
+            offset(construct_offset(options[:offset], scope))
         end
 
         def construct_finder_sql(options, scope = scope(:find))
@@ -1810,7 +1810,7 @@ module ActiveRecord #:nodoc:
 
         def build_association_joins(joins)
           join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, joins, nil)
-          relation = arel_table
+          relation = arel_table.relation
           join_dependency.join_associations.map { |association|
             if association.relation.is_a?(Array)
               [Arel::InnerJoin.new(relation, association.relation.first, association.association_join.first).joins(relation),
@@ -2569,7 +2569,7 @@ module ActiveRecord #:nodoc:
       # be made (since they can't be persisted).
       def destroy
         unless new_record?
-          arel_table(true).conditions!(arel_table[self.class.primary_key].eq(id)).delete
+          arel_table(true).conditions(arel_table[self.class.primary_key].eq(id)).delete
         end
 
         @destroyed = true
@@ -2864,7 +2864,7 @@ module ActiveRecord #:nodoc:
       def update(attribute_names = @attributes.keys)
         attributes_with_values = arel_attributes_values(false, false, attribute_names)
         return 0 if attributes_with_values.empty?
-        arel_table(true).conditions!(arel_table[self.class.primary_key].eq(id)).update(attributes_with_values)
+        arel_table(true).conditions(arel_table[self.class.primary_key].eq(id)).update(attributes_with_values)
       end
 
       # Creates a record with values matching those of the instance attributes
@@ -2952,7 +2952,7 @@ module ActiveRecord #:nodoc:
       end
 
       def arel_table(reload = nil)
-        @arel_table = Relation.new(self.class, self.class.table_name) if reload || @arel_table.nil?
+        @arel_table = Relation.new(self.class, Arel::Table.new(self.class.table_name)) if reload || @arel_table.nil?
         @arel_table
       end
 
diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index cd2fbe9b79..40242333e5 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -147,11 +147,11 @@ module ActiveRecord
             construct_finder_arel_with_included_associations(options, join_dependency)
           else
             relation = arel_table(options[:from]).
-              joins!(construct_join(options[:joins], scope)).
-              conditions!(construct_conditions(options[:conditions], scope)).
-              order!(options[:order]).
-              limit!(options[:limit]).
-              offset!(options[:offset])
+              joins(construct_join(options[:joins], scope)).
+              conditions(construct_conditions(options[:conditions], scope)).
+              order(options[:order]).
+              limit(options[:limit]).
+              offset(options[:offset])
           end
           if options[:group]
             return execute_grouped_calculation(operation, column_name, options, relation)
@@ -171,7 +171,7 @@ module ActiveRecord
                                (column_name == :all ? "*" : column_name.to_s))
         end
 
-        relation.select!(operation == 'count' ? column.count(options[:distinct]) : column.send(operation))
+        relation = relation.select(operation == 'count' ? column.count(options[:distinct]) : column.send(operation))
 
         type_cast_calculated_value(connection.select_value(relation.to_sql), column_for(column_name), operation)
       end
@@ -194,8 +194,7 @@ module ActiveRecord
 
         options[:select] <<  ", #{group_field} AS #{group_alias}"
 
-        relation.select!(options[:select])
-        relation.group!(construct_group(options[:group], options[:having], nil))
+        relation = relation.select(options[:select]).group(construct_group(options[:group], options[:having], nil))
 
         calculated_data = connection.select_all(relation.to_sql)
 
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 9c7ba881fc..bba0d0844d 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -3,9 +3,9 @@ module ActiveRecord
     delegate :to_sql, :to => :relation
     attr_reader :relation, :klass
 
-    def initialize(klass, table = nil)
+    def initialize(klass, relation)
       @klass = klass
-      @relation = Arel::Table.new(table || @klass.table_name)
+      @relation = relation
     end
 
     def to_a
@@ -21,60 +21,57 @@ module ActiveRecord
       to_a.first
     end
 
-    def select!(selection)
-      @relation = @relation.project(selection) if selection
-      self
+    def select(selects)
+      Relation.new(@klass, @relation.project(selects))
     end
 
-    def on!(on)
-      @relation = @relation.on(on) if on
-      self
+    def group(groups)
+      Relation.new(@klass, @relation.group(groups))
     end
 
-    def order!(order)
-      @relation = @relation.order(order) if order
-      self
+    def order(orders)
+      Relation.new(@klass, @relation.order(orders))
     end
 
-    def group!(group)
-      @relation = @relation.group(group) if group
-      self
+    def limit(limits)
+      Relation.new(@klass, @relation.take(limits))
     end
 
-    def limit!(limit)
-      @relation = @relation.take(limit) if limit
-      self
+    def offset(offsets)
+      Relation.new(@klass, @relation.skip(offsets))
     end
 
-    def offset!(offset)
-      @relation = @relation.skip(offset) if offset
-      self
+    def on(join)
+      Relation.new(@klass, @relation.on(join))
     end
 
-    def joins!(joins, join_type = nil)
-      if !joins.blank?
-        @relation = case joins
-        when String
-          @relation.join(joins)
-        when Hash, Array, Symbol
-          if @klass.send(:array_of_strings?, joins)
-            @relation.join(joins.join(' '))
+    def joins(join, join_type = nil)
+      if join.blank?
+        self
+      else
+        join = case join
+          when String
+            @relation.join(join)
+          when Hash, Array, Symbol
+            if @klass.send(:array_of_strings?, join)
+              @relation.join(join.join(' '))
+            else
+              @relation.join(@klass.send(:build_association_joins, join))
+            end
           else
-            @relation.join(@klass.send(:build_association_joins, joins))
-          end
-        else
-          @relation.join(joins, join_type)
+            @relation.join(join, join_type)
         end
+        Relation.new(@klass, join)
       end
-      self
     end
 
-    def conditions!(conditions)
-      if !conditions.blank?
+    def conditions(conditions)
+      if conditions.blank?
+        self
+      else
         conditions = @klass.send(:merge_conditions, conditions) if [String, Hash, Array].include?(conditions.class)
-        @relation = @relation.where(conditions)
+        Relation.new(@klass, @relation.where(conditions))
       end
-      self
     end
 
     private
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index b4f29f939a..df15c1a797 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1893,7 +1893,7 @@ class BasicsTest < ActiveRecord::TestCase
   end
 
   def test_all_with_conditions
-    assert_equal Developer.find(:all, :order => 'id desc'), Developer.all.order!('id desc').to_a
+    assert_equal Developer.find(:all, :order => 'id desc'), Developer.all.order('id desc').to_a
   end
 
   def test_find_ordered_last
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index e8b222f876..b017570b45 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -11,47 +11,47 @@ class RelationTest < ActiveRecord::TestCase
   fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts
 
   def test_finding_with_conditions
-    assert_equal Author.find(:all, :conditions => "name = 'David'"), Author.all.conditions!("name = 'David'").to_a
+    assert_equal Author.find(:all, :conditions => "name = 'David'"), Author.all.conditions("name = 'David'").to_a
   end
 
   def test_finding_with_order
-    topics = Topic.all.order!('id')
+    topics = Topic.all.order('id')
     assert_equal 4, topics.size
     assert_equal topics(:first).title, topics.first.title
   end
 
   def test_finding_with_order_and_take
-    entrants = Entrant.all.order!("id ASC").limit!(2).to_a
+    entrants = Entrant.all.order("id ASC").limit(2).to_a
 
     assert_equal(2, entrants.size)
     assert_equal(entrants(:first).name, entrants.first.name)
   end
 
   def test_finding_with_order_limit_and_offset
-    entrants = Entrant.all.order!("id ASC").limit!(2).offset!(1)
+    entrants = Entrant.all.order("id ASC").limit(2).offset(1)
 
     assert_equal(2, entrants.size)
     assert_equal(entrants(:second).name, entrants.first.name)
 
-    entrants = Entrant.all.order!("id ASC").limit!(2).offset!(2)
+    entrants = Entrant.all.order("id ASC").limit(2).offset(2)
     assert_equal(1, entrants.size)
     assert_equal(entrants(:third).name, entrants.first.name)
   end
 
   def test_finding_with_group
-    developers = Developer.all.group!("salary").select!("salary").to_a
+    developers = Developer.all.group("salary").select("salary").to_a
     assert_equal 4, developers.size
     assert_equal 4, developers.map(&:salary).uniq.size
   end
 
   def test_finding_with_hash_conditions_on_joined_table
-    firms = DependentFirm.all.joins!(:account).conditions!({:name => 'RailsCore', :accounts => { :credit_limit => 55..60 }}).to_a
+    firms = DependentFirm.all.joins(:account).conditions({:name => 'RailsCore', :accounts => { :credit_limit => 55..60 }}).to_a
     assert_equal 1, firms.size
     assert_equal companies(:rails_core), firms.first
   end
 
   def test_find_all_with_join
-    developers_on_project_one = Developer.all.joins!('LEFT JOIN developers_projects ON developers.id = developers_projects.developer_id').conditions!('project_id=1').to_a
+    developers_on_project_one = Developer.all.joins('LEFT JOIN developers_projects ON developers.id = developers_projects.developer_id').conditions('project_id=1').to_a
 
     assert_equal 3, developers_on_project_one.length
     developer_names = developers_on_project_one.map { |d| d.name }
@@ -60,11 +60,11 @@ class RelationTest < ActiveRecord::TestCase
   end
 
   def test_find_on_hash_conditions
-    assert_equal Topic.find(:all, :conditions => {:approved => false}), Topic.all.conditions!({ :approved => false }).to_a
+    assert_equal Topic.find(:all, :conditions => {:approved => false}), Topic.all.conditions({ :approved => false }).to_a
   end
 
   def test_joins_with_string_array
-    person_with_reader_and_post = Post.all.joins!([
+    person_with_reader_and_post = Post.all.joins([
         "INNER JOIN categorizations ON categorizations.post_id = posts.id",
         "INNER JOIN categories ON categories.id = categorizations.category_id AND categories.type = 'SpecialCategory'"
       ]