Inject the PredicateBuilder into the Relation instance

Construction of relations can be a hotspot, we don't want to create one
of these in the constructor. This also allows us to do more expensive
things in the predicate builder's constructor, since it's created once
per AR::Base subclass

activerecord/lib/active_record/association_relation.rb

@@ -1,7 +1,7 @@
 module ActiveRecord
   class AssociationRelation < Relation
-    def initialize(klass, table, association)
-      super(klass, table)
+    def initialize(klass, table, predicate_builder, association)
+      super(klass, table, predicate_builder)
       @association = association
     end
 

activerecord/lib/active_record/associations/association.rb

@@ -121,7 +121,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, self).merge!(klass.all)
+        AssociationRelation.create(klass, klass.arel_table, klass.predicate_builder, self).merge!(klass.all)
       end
 
       # Loads the \target if needed and returns it.

activerecord/lib/active_record/associations/collection_proxy.rb

@@ -32,7 +32,7 @@ module ActiveRecord
 
       def initialize(klass, association) #:nodoc:
         @association = association
-        super klass, klass.arel_table
+        super klass, klass.arel_table, klass.predicate_builder
         merge! association.scope(nullify: false)
       end
 

activerecord/lib/active_record/associations/join_dependency/join_association.rb

@@ -43,16 +43,23 @@ module ActiveRecord
 
             constraint = build_constraint(klass, table, key, foreign_table, foreign_key)
 
+            predicate_builder = PredicateBuilder.new(klass, table)
             scope_chain_items = scope_chain[scope_chain_index].map do |item|
               if item.is_a?(Relation)
                 item
               else
-                ActiveRecord::Relation.create(klass, table).instance_exec(node, &item)
+                ActiveRecord::Relation.create(klass, table, predicate_builder)
+                  .instance_exec(node, &item)
               end
             end
             scope_chain_index += 1
 
-            scope_chain_items.concat [klass.send(:build_default_scope, ActiveRecord::Relation.create(klass, table))].compact
+            relation = ActiveRecord::Relation.create(
+              klass,
+              table,
+              predicate_builder,
+            )
+            scope_chain_items.concat [klass.send(:build_default_scope, relation)].compact
 
             rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right|
               left.merge right

activerecord/lib/active_record/core.rb

@@ -248,10 +248,14 @@ module ActiveRecord
           end
       end
 
+      def predicate_builder # :nodoc:
+        @predicate_builder ||= PredicateBuilder.new(self, arel_table)
+      end
+
       private
 
       def relation #:nodoc:
-        relation = Relation.create(self, arel_table)
+        relation = Relation.create(self, arel_table, predicate_builder)
 
         if finder_needs_type_condition?
           relation.where(type_condition).create_with(inheritance_column.to_sym => sti_name)

activerecord/lib/active_record/model_schema.rb

@@ -147,6 +147,7 @@ module ActiveRecord
         @quoted_table_name = nil
         @arel_table        = nil
         @sequence_name     = nil unless defined?(@explicit_sequence_name) && @explicit_sequence_name
+        @predicate_builder = nil
       end
 
       # Returns a quoted version of the table name, used to construct SQL statements.

activerecord/lib/active_record/relation.rb

@@ -16,17 +16,17 @@ module ActiveRecord
 
     include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation
 
-    attr_reader :table, :klass, :loaded
+    attr_reader :table, :klass, :loaded, :predicate_builder
     alias :model :klass
     alias :loaded? :loaded
 
-    def initialize(klass, table, values = {})
+    def initialize(klass, table, predicate_builder, values = {})
       @klass  = klass
       @table  = table
       @values = values
       @offsets = {}
       @loaded = false
-      @predicate_builder = PredicateBuilder.new(klass, table)
+      @predicate_builder = predicate_builder
     end
 
     def initialize_copy(other)
@@ -633,10 +633,6 @@ module ActiveRecord
       "#<#{self.class.name} [#{entries.join(', ')}]>"
     end
 
-    protected
-
-    attr_reader :predicate_builder
-
     private
 
     def exec_queries

activerecord/lib/active_record/relation/merger.rb

@@ -22,7 +22,7 @@ module ActiveRecord
       # build a relation to merge in rather than directly merging
       # the values.
       def other
-        other = Relation.create(relation.klass, relation.table)
+        other = Relation.create(relation.klass, relation.table, relation.predicate_builder)
         hash.each { |k, v|
           if k == :joins
             if Hash === v

activerecord/lib/active_record/relation/spawn_methods.rb

@@ -67,7 +67,7 @@ module ActiveRecord
     private
 
       def relation_with(values) # :nodoc:
-        result = Relation.create(klass, table, values)
+        result = Relation.create(klass, table, predicate_builder, values)
         result.extend(*extending_values) if extending_values.any?
         result
       end

activerecord/test/cases/relation/mutation_test.rb

@@ -25,7 +25,7 @@ module ActiveRecord
     end
 
     def relation
-      @relation ||= Relation.new FakeKlass.new('posts'), Post.arel_table
+      @relation ||= Relation.new FakeKlass.new('posts'), Post.arel_table, Post.predicate_builder
     end
 
     (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :unscope, :select]).each do |method|

activerecord/test/cases/relation_test.rb

@@ -23,19 +23,19 @@ module ActiveRecord
     end
 
     def test_construction
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       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
+      relation = Relation.new(FakeKlass, :b, nil)
       assert_equal FakeKlass, relation.model
     end
 
     def test_initialize_single_values
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method|
         assert_nil relation.send("#{method}_value"), method.to_s
       end
@@ -43,19 +43,19 @@ module ActiveRecord
     end
 
     def test_multi_value_initialize
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       Relation::MULTI_VALUE_METHODS.each do |method|
         assert_equal [], relation.send("#{method}_values"), method.to_s
       end
     end
 
     def test_extensions
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       assert_equal [], relation.extensions
     end
 
     def test_empty_where_values_hash
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       assert_equal({}, relation.where_values_hash)
 
       relation.where! :hello
@@ -63,19 +63,19 @@ module ActiveRecord
     end
 
     def test_has_values
-      relation = Relation.new Post, Post.arel_table
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
       relation.where! relation.table[:id].eq(10)
       assert_equal({:id => 10}, relation.where_values_hash)
     end
 
     def test_values_wrong_table
-      relation = Relation.new Post, Post.arel_table
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
       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
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
       left     = relation.table[:id].eq(10)
       right    = relation.table[:id].eq(10)
       combine  = left.and right
@@ -84,24 +84,24 @@ module ActiveRecord
     end
 
     def test_table_name_delegates_to_klass
-      relation = Relation.new FakeKlass.new('posts'), :b
+      relation = Relation.new(FakeKlass.new('posts'), :b, Post.predicate_builder)
       assert_equal 'posts', relation.table_name
     end
 
     def test_scope_for_create
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       assert_equal({}, relation.scope_for_create)
     end
 
     def test_create_with_value
-      relation = Relation.new Post, Post.arel_table
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
       hash = { :hello => 'world' }
       relation.create_with_value = hash
       assert_equal hash, relation.scope_for_create
     end
 
     def test_create_with_value_with_wheres
-      relation = Relation.new Post, Post.arel_table
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
       relation.where! relation.table[:id].eq(10)
       relation.create_with_value = {:hello => 'world'}
       assert_equal({:hello => 'world', :id => 10}, relation.scope_for_create)
@@ -109,7 +109,7 @@ module ActiveRecord
 
     # FIXME: is this really wanted or expected behavior?
     def test_scope_for_create_is_cached
-      relation = Relation.new Post, Post.arel_table
+      relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
       assert_equal({}, relation.scope_for_create)
 
       relation.where! relation.table[:id].eq(10)
@@ -126,31 +126,31 @@ module ActiveRecord
     end
 
     def test_empty_eager_loading?
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       assert !relation.eager_loading?
     end
 
     def test_eager_load_values
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       relation.eager_load! :b
       assert relation.eager_loading?
     end
 
     def test_references_values
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       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
+      relation = Relation.new(FakeKlass, :b, nil)
       relation = relation.references(:foo).references(:foo)
       assert_equal ['foo'], relation.references_values
     end
 
     test 'merging a hash into a relation' do
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       relation = relation.merge where: :lol, readonly: true
 
       assert_equal [:lol], relation.where_values
@@ -158,7 +158,7 @@ module ActiveRecord
     end
 
     test 'merging an empty hash into a relation' do
-      assert_equal [], Relation.new(FakeKlass, :b).merge({}).where_values
+      assert_equal [], Relation.new(FakeKlass, :b, nil).merge({}).where_values
     end
 
     test 'merging a hash with unknown keys raises' do
@@ -166,7 +166,7 @@ module ActiveRecord
     end
 
     test '#values returns a dup of the values' do
-      relation = Relation.new(FakeKlass, :b).where! :foo
+      relation = Relation.new(FakeKlass, :b, nil).where! :foo
       values   = relation.values
 
       values[:where] = nil
@@ -174,12 +174,12 @@ module ActiveRecord
     end
 
     test 'relations can be created with a values hash' do
-      relation = Relation.new(FakeKlass, :b, where: [:foo])
+      relation = Relation.new(FakeKlass, :b, nil, where: [:foo])
       assert_equal [:foo], relation.where_values
     end
 
     test 'merging a single where value' do
-      relation = Relation.new(FakeKlass, :b)
+      relation = Relation.new(FakeKlass, :b, nil)
       relation.merge!(where: :foo)
       assert_equal [:foo], relation.where_values
     end
@@ -192,13 +192,13 @@ module ActiveRecord
         end
       end
 
-      relation = Relation.new(klass, :b)
+      relation = Relation.new(klass, :b, nil)
       relation.merge!(where: ['foo = ?', 'bar'])
       assert_equal ['foo = bar'], relation.where_values
     end
 
     def test_merging_readonly_false
-      relation = Relation.new FakeKlass, :b
+      relation = Relation.new(FakeKlass, :b, nil)
       readonly_false_relation = relation.readonly(false)
       # test merging in both directions
       assert_equal false, relation.merge(readonly_false_relation).readonly_value

activerecord/test/cases/relations_test.rb

@@ -1663,7 +1663,8 @@ class RelationTest < ActiveRecord::TestCase
   test 'using a custom table affects the wheres' do
     table_alias = Post.arel_table.alias('omg_posts')
 
-    relation = ActiveRecord::Relation.new Post, table_alias
+    predicate_builder = ActiveRecord::PredicateBuilder.new(Post, table_alias)
+    relation = ActiveRecord::Relation.new(Post, table_alias, predicate_builder)
     relation.where!(:foo => "bar")
 
     node = relation.arel.constraints.first.grep(Arel::Attributes::Attribute).first