Avoid passing unnecessary arguments to relation

Most of the time the table and predicate_builder
passed to Relation.new are exactly the
arel_table and predicate builder of the
given klass. This uses klass.arel_table
and klass.predicate_builder as the defaults,
so we don't have to pass them in most cases.

This does change the signaure of both Relation and
AssocationRelation. Are we ok with that?

14 files changed, 66 insertions, 38 deletions

diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb
index 2b0b2864bc..403667fb70 100644
--- a/activerecord/lib/active_record/association_relation.rb
+++ b/activerecord/lib/active_record/association_relation.rb
@@ -2,8 +2,8 @@
 
 module ActiveRecord
   class AssociationRelation < Relation
-    def initialize(klass, table, predicate_builder, association)
-      super(klass, table, predicate_builder)
+    def initialize(klass, association)
+      super(klass)
       @association = association
     end
 
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index ca1f9f1650..7667c6ed8b 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -124,7 +124,7 @@ module ActiveRecord
       # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
       # through association's scope)
       def target_scope
-        AssociationRelation.create(klass, klass.arel_table, klass.predicate_builder, self).merge!(klass.all)
+        AssociationRelation.create(klass, self).merge!(klass.all)
       end
 
       def extensions
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 8b4a48a38c..9a30198b95 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -32,7 +32,7 @@ module ActiveRecord
     class CollectionProxy < Relation
       def initialize(klass, association) #:nodoc:
         @association = association
-        super klass, klass.arel_table, klass.predicate_builder
+        super klass
 
         extensions = association.extensions
         extend(*extensions) if extensions.any?
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 88810cb328..2c65f618dc 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -281,7 +281,7 @@ module ActiveRecord
         end
 
         def relation
-          relation = Relation.create(self, arel_table, predicate_builder)
+          relation = Relation.create(self)
 
           if finder_needs_type_condition? && !ignore_default_scope?
             relation.where!(type_condition)
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index c28e31a3da..e640d75d2f 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -291,7 +291,11 @@ module ActiveRecord
       end
 
       def build_scope(table, predicate_builder = predicate_builder(table))
-        Relation.create(klass, table, predicate_builder)
+        Relation.create(
+          klass,
+          table: table,
+          predicate_builder: predicate_builder
+        )
       end
 
       def join_primary_key(*)
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 10be583ef4..d71c430045 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -22,7 +22,7 @@ module ActiveRecord
     alias :loaded? :loaded
     alias :locked? :lock_value
 
-    def initialize(klass, table, predicate_builder, values = {})
+    def initialize(klass, table: klass.arel_table, predicate_builder: klass.predicate_builder, values: {})
       @klass  = klass
       @table  = table
       @values = values
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index ebdd4144bb..25510d4a57 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -23,7 +23,11 @@ module ActiveRecord
       # build a relation to merge in rather than directly merging
       # the values.
       def other
-        other = Relation.create(relation.klass, relation.table, relation.predicate_builder)
+        other = Relation.create(
+          relation.klass,
+          table: relation.table,
+          predicate_builder: relation.predicate_builder
+        )
         hash.each { |k, v|
           if k == :joins
             if Hash === v
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index 617d8de8b2..562e04194c 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -69,7 +69,7 @@ module ActiveRecord
     private
 
       def relation_with(values)
-        result = Relation.create(klass, table, predicate_builder, values)
+        result = Relation.create(klass, values: values)
         result.extend(*extending_values) if extending_values.any?
         result
       end
diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb
index be8aeed5ac..bbd0b1724a 100644
--- a/activerecord/test/cases/batches_test.rb
+++ b/activerecord/test/cases/batches_test.rb
@@ -592,7 +592,11 @@ class EachTest < ActiveRecord::TestCase
       table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias)
       predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata)
 
-      posts = ActiveRecord::Relation.create(Post, table_alias, predicate_builder)
+      posts = ActiveRecord::Relation.create(
+        Post,
+        table: table_alias,
+        predicate_builder: predicate_builder
+      )
       posts.find_each {}
     end
   end
diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb
index cfe95b2360..a5d908344a 100644
--- a/activerecord/test/cases/collection_cache_key_test.rb
+++ b/activerecord/test/cases/collection_cache_key_test.rb
@@ -60,7 +60,11 @@ module ActiveRecord
       table_metadata = ActiveRecord::TableMetadata.new(Developer, table_alias)
       predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata)
 
-      developers = ActiveRecord::Relation.create(Developer, table_alias, predicate_builder)
+      developers = ActiveRecord::Relation.create(
+        Developer,
+        table: table_alias,
+        predicate_builder: predicate_builder
+      )
       developers = developers.where(salary: 100000).order(updated_at: :desc)
       last_developer_timestamp = developers.first.updated_at
 
diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb
index ad3700b73a..932a4eca2c 100644
--- a/activerecord/test/cases/relation/mutation_test.rb
+++ b/activerecord/test/cases/relation/mutation_test.rb
@@ -139,7 +139,7 @@ module ActiveRecord
 
     private
       def relation
-        @relation ||= Relation.new(FakeKlass, Post.arel_table, Post.predicate_builder)
+        @relation ||= Relation.new(FakeKlass)
       end
   end
 end
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index dbf3389774..3d64bfb810 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -11,19 +11,19 @@ module ActiveRecord
     fixtures :posts, :comments, :authors, :author_addresses, :ratings
 
     def test_construction
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass, table: :b)
       assert_equal FakeKlass, relation.klass
       assert_equal :b, relation.table
       assert !relation.loaded, "relation is not loaded"
     end
 
     def test_responds_to_model_and_returns_klass
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       assert_equal FakeKlass, relation.model
     end
 
     def test_initialize_single_values
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method|
         assert_nil relation.send("#{method}_value"), method.to_s
       end
@@ -33,7 +33,7 @@ module ActiveRecord
     end
 
     def test_multi_value_initialize
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       Relation::MULTI_VALUE_METHODS.each do |method|
         values = relation.send("#{method}_values")
         assert_equal [], values, method.to_s
@@ -42,29 +42,29 @@ module ActiveRecord
     end
 
     def test_extensions
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       assert_equal [], relation.extensions
     end
 
     def test_empty_where_values_hash
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       assert_equal({}, relation.where_values_hash)
     end
 
     def test_has_values
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       relation.where!(id: 10)
       assert_equal({ "id" => 10 }, relation.where_values_hash)
     end
 
     def test_values_wrong_table
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       relation.where! Comment.arel_table[:id].eq(10)
       assert_equal({}, relation.where_values_hash)
     end
 
     def test_tree_is_not_traversed
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       left     = relation.table[:id].eq(10)
       right    = relation.table[:id].eq(10)
       combine  = left.or(right)
@@ -73,18 +73,18 @@ module ActiveRecord
     end
 
     def test_scope_for_create
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       assert_equal({}, relation.scope_for_create)
     end
 
     def test_create_with_value
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       relation.create_with_value = { hello: "world" }
       assert_equal({ "hello" => "world" }, relation.scope_for_create)
     end
 
     def test_create_with_value_with_wheres
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       assert_equal({}, relation.scope_for_create)
 
       relation.where!(id: 10)
@@ -95,7 +95,7 @@ module ActiveRecord
     end
 
     def test_empty_scope
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       assert relation.empty_scope?
 
       relation.merge!(relation)
@@ -109,31 +109,31 @@ module ActiveRecord
     end
 
     def test_empty_eager_loading?
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       assert !relation.eager_loading?
     end
 
     def test_eager_load_values
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       relation.eager_load! :b
       assert relation.eager_loading?
     end
 
     def test_references_values
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       assert_equal [], relation.references_values
       relation = relation.references(:foo).references(:omg, :lol)
       assert_equal ["foo", "omg", "lol"], relation.references_values
     end
 
     def test_references_values_dont_duplicate
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       relation = relation.references(:foo).references(:foo)
       assert_equal ["foo"], relation.references_values
     end
 
     test "merging a hash into a relation" do
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
+      relation = Relation.new(Post)
       relation = relation.merge where: { name: :lol }, readonly: true
 
       assert_equal({ "name" => :lol }, relation.where_clause.to_h)
@@ -141,7 +141,7 @@ module ActiveRecord
     end
 
     test "merging an empty hash into a relation" do
-      assert_equal Relation::WhereClause.empty, Relation.new(FakeKlass, :b, nil).merge({}).where_clause
+      assert_equal Relation::WhereClause.empty, Relation.new(FakeKlass).merge({}).where_clause
     end
 
     test "merging a hash with unknown keys raises" do
@@ -149,7 +149,7 @@ module ActiveRecord
     end
 
     test "merging nil or false raises" do
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
 
       e = assert_raises(ArgumentError) do
         relation = relation.merge nil
@@ -165,7 +165,7 @@ module ActiveRecord
     end
 
     test "#values returns a dup of the values" do
-      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder).where!(name: :foo)
+      relation = Relation.new(Post).where!(name: :foo)
       values   = relation.values
 
       values[:where] = nil
@@ -173,7 +173,7 @@ module ActiveRecord
     end
 
     test "relations can be created with a values hash" do
-      relation = Relation.new(FakeKlass, :b, nil, select: [:foo])
+      relation = Relation.new(FakeKlass, values: { select: [:foo] })
       assert_equal [:foo], relation.select_values
     end
 
@@ -185,13 +185,13 @@ module ActiveRecord
         end
       end
 
-      relation = Relation.new(klass, :b, nil)
+      relation = Relation.new(klass)
       relation.merge!(where: ["foo = ?", "bar"])
       assert_equal Relation::WhereClause.new(["foo = bar"]), relation.where_clause
     end
 
     def test_merging_readonly_false
-      relation = Relation.new(FakeKlass, :b, nil)
+      relation = Relation.new(FakeKlass)
       readonly_false_relation = relation.readonly(false)
       # test merging in both directions
       assert_equal false, relation.merge(readonly_false_relation).readonly_value
@@ -235,7 +235,7 @@ module ActiveRecord
 
     def test_merge_raises_with_invalid_argument
       assert_raises ArgumentError do
-        relation = Relation.new(FakeKlass, :b, nil)
+        relation = Relation.new(FakeKlass)
         relation.merge(true)
       end
     end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 7785f8c99b..19489f3c71 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1934,6 +1934,10 @@ class RelationTest < ActiveRecord::TestCase
       table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias)
       predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata)
 
-      ActiveRecord::Relation.create(Post, table_alias, predicate_builder)
+      ActiveRecord::Relation.create(
+        Post,
+        table: table_alias,
+        predicate_builder: predicate_builder
+      )
     end
 end
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index b552f66787..54eb5e6783 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -326,5 +326,13 @@ class FakeKlass
     def enforce_raw_sql_whitelist(*args)
       # noop
     end
+
+    def arel_table
+      Post.arel_table
+    end
+
+    def predicate_builder
+      Post.predicate_builder
+    end
   end
 end