Skip to content

Commit

Permalink
Merge pull request #30050 from kamipo/dont_pass_connection_to_stateme…
Browse files Browse the repository at this point in the history
…nt_cache_execute

Passing `klass` to `StatementCache.new`
  • Loading branch information
sgrif committed Aug 3, 2017
2 parents ec54435 + 510428f commit 7d69903
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -304,15 +304,13 @@ def find_target
return scope.to_a if skip_statement_cache?(scope)

conn = klass.connection
sc = reflection.association_scope_cache(conn, owner) do
StatementCache.create(conn) { |params|
as = AssociationScope.create { params.bind }
target_scope.merge!(as.scope(self))
}
sc = reflection.association_scope_cache(conn, owner) do |params|
as = AssociationScope.create { params.bind }
target_scope.merge!(as.scope(self))
end

binds = AssociationScope.get_bind_values(owner, reflection.chain)
sc.execute(binds, klass, conn) do |record|
sc.execute(binds, conn) do |record|
set_inverse_instance(record)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,13 @@ def find_target
return scope.take if skip_statement_cache?(scope)

conn = klass.connection
sc = reflection.association_scope_cache(conn, owner) do
StatementCache.create(conn) { |params|
as = AssociationScope.create { params.bind }
target_scope.merge!(as.scope(self)).limit(1)
}
sc = reflection.association_scope_cache(conn, owner) do |params|
as = AssociationScope.create { params.bind }
target_scope.merge!(as.scope(self)).limit(1)
end

binds = AssociationScope.get_bind_values(owner, reflection.chain)
sc.execute(binds, klass, conn) do |record|
sc.execute(binds, conn) do |record|
set_inverse_instance record
end.first
rescue ::RangeError
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def find(*ids) # :nodoc:
where(key => params.bind).limit(1)
}

record = statement.execute([id], self, connection).first
record = statement.execute([id], connection).first
unless record
raise RecordNotFound.new("Couldn't find #{name} with '#{primary_key}'=#{id}",
name, primary_key, id)
Expand Down Expand Up @@ -208,7 +208,7 @@ def find_by(*args) # :nodoc:
where(wheres).limit(1)
}
begin
statement.execute(hash.values, self, connection).first
statement.execute(hash.values, connection).first
rescue TypeError
raise ActiveRecord::StatementInvalid
rescue ::RangeError
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,13 @@ def initialize(name, scope, options, active_record)
end
end

def association_scope_cache(conn, owner)
def association_scope_cache(conn, owner, &block)
key = conn.prepared_statements
if polymorphic?
key = [key, owner._read_attribute(@foreign_type)]
end
@association_scope_cache[key] ||= @scope_lock.synchronize {
@association_scope_cache[key] ||= yield
@association_scope_cache[key] ||= StatementCache.create(conn, &block)
}
end

Expand Down
19 changes: 11 additions & 8 deletions activerecord/lib/active_record/statement_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module ActiveRecord
# The cached statement is executed by using the
# {connection.execute}[rdoc-ref:ConnectionAdapters::DatabaseStatements#execute] method:
#
# cache.execute([], Book, Book.connection)
# cache.execute([], Book.connection)
#
# The relation returned by the block is cached, and for each
# {execute}[rdoc-ref:ConnectionAdapters::DatabaseStatements#execute]
Expand All @@ -26,7 +26,7 @@ module ActiveRecord
#
# And pass the bind values as the first argument of +execute+ call.
#
# cache.execute(["my book"], Book, Book.connection)
# cache.execute(["my book"], Book.connection)
class StatementCache # :nodoc:
class Substitute; end # :nodoc:

Expand Down Expand Up @@ -87,21 +87,20 @@ def bind(values)
end
end

attr_reader :bind_map, :query_builder

def self.create(connection, block = Proc.new)
relation = block.call Params.new
query_builder, binds = connection.cacheable_query(self, relation.arel)
bind_map = BindMap.new(binds)
new query_builder, bind_map
new(query_builder, bind_map, relation.klass)
end

def initialize(query_builder, bind_map)
def initialize(query_builder, bind_map, klass)
@query_builder = query_builder
@bind_map = bind_map
@bind_map = bind_map
@klass = klass
end

def execute(params, klass, connection, &block)
def execute(params, connection, &block)
bind_values = bind_map.bind params

sql = query_builder.sql_for bind_values, connection
Expand All @@ -114,5 +113,9 @@ def self.unsupported_value?(value)
when NilClass, Array, Range, Hash, Relation, Base then true
end
end

protected

attr_reader :query_builder, :bind_map, :klass
end
end
16 changes: 8 additions & 8 deletions activerecord/test/cases/statement_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ def test_statement_cache
Book.where(name: params.bind)
end

b = cache.execute([ "my book" ], Book, Book.connection)
b = cache.execute([ "my book" ], Book.connection)
assert_equal "my book", b[0].name
b = cache.execute([ "my other book" ], Book, Book.connection)
b = cache.execute([ "my other book" ], Book.connection)
assert_equal "my other book", b[0].name
end

Expand All @@ -35,9 +35,9 @@ def test_statement_cache_id
Book.where(id: params.bind)
end

b = cache.execute([ b1.id ], Book, Book.connection)
b = cache.execute([ b1.id ], Book.connection)
assert_equal b1.name, b[0].name
b = cache.execute([ b2.id ], Book, Book.connection)
b = cache.execute([ b2.id ], Book.connection)
assert_equal b2.name, b[0].name
end

Expand All @@ -60,7 +60,7 @@ def test_statement_cache_with_simple_statement

Book.create(name: "my book", author_id: 4)

books = cache.execute([], Book, Book.connection)
books = cache.execute([], Book.connection)
assert_equal "my book", books[0].name
end

Expand All @@ -73,7 +73,7 @@ def test_statement_cache_with_complex_statement
molecule = salty.molecules.create(name: "dioxane")
molecule.electrons.create(name: "lepton")

liquids = cache.execute([], Book, Book.connection)
liquids = cache.execute([], Book.connection)
assert_equal "salty", liquids[0].name
end

Expand All @@ -86,13 +86,13 @@ def test_statement_cache_values_differ
Book.create(name: "my book")
end

first_books = cache.execute([], Book, Book.connection)
first_books = cache.execute([], Book.connection)

3.times do
Book.create(name: "my book")
end

additional_books = cache.execute([], Book, Book.connection)
additional_books = cache.execute([], Book.connection)
assert first_books != additional_books
end

Expand Down

0 comments on commit 7d69903

Please sign in to comment.