Merge pull request #51353 from Shopify/get-rid-lease-connection

Eliminate remaining uses of `lease_connection` inside Active Record
This commit is contained in:
Jean Boussier 2024-03-19 18:03:37 +01:00 committed by GitHub
commit 7c68c5210c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 72 additions and 67 deletions

View File

@ -37,6 +37,7 @@ module ActiveRecord
chain << [reflection, table] chain << [reflection, table]
end end
base_klass.with_connection do |connection|
# The chain starts with the target table, but we want to end with it here (makes # The chain starts with the target table, but we want to end with it here (makes
# more sense in this context), so we reverse # more sense in this context), so we reverse
chain.reverse_each do |reflection, table| chain.reverse_each do |reflection, table|
@ -65,12 +66,13 @@ module ActiveRecord
if others && !others.empty? if others && !others.empty?
joins.concat arel.join_sources joins.concat arel.join_sources
append_constraints(joins.last, others) append_constraints(connection, joins.last, others)
end end
# The current table in this iteration becomes the foreign table in the next # The current table in this iteration becomes the foreign table in the next
foreign_table, foreign_klass = table, klass foreign_table, foreign_klass = table, klass
end end
end
joins joins
end end
@ -88,10 +90,10 @@ module ActiveRecord
end end
private private
def append_constraints(join, constraints) def append_constraints(connection, join, constraints)
if join.is_a?(Arel::Nodes::StringJoin) if join.is_a?(Arel::Nodes::StringJoin)
join_string = Arel::Nodes::And.new(constraints.unshift join.left) join_string = Arel::Nodes::And.new(constraints.unshift join.left)
join.left = Arel.sql(base_klass.lease_connection.visitor.compile(join_string)) join.left = Arel.sql(connection.visitor.compile(join_string))
else else
right = join.right right = join.right
right.expr = Arel::Nodes::And.new(constraints.unshift right.expr) right.expr = Arel::Nodes::And.new(constraints.unshift right.expr)

View File

@ -178,7 +178,10 @@ module ActiveRecord
def can_use_fast_cache_version?(timestamp) def can_use_fast_cache_version?(timestamp)
timestamp.is_a?(String) && timestamp.is_a?(String) &&
cache_timestamp_format == :usec && cache_timestamp_format == :usec &&
self.class.lease_connection.default_timezone == :utc && # FIXME: checking out a connection for this is wasteful
# we should store/cache this information in the schema cache
# or similar.
self.class.with_connection(&:default_timezone) == :utc &&
!updated_at_came_from_user? !updated_at_came_from_user?
end end

View File

@ -44,7 +44,7 @@ module ActiveRecord
arel_table = arel_table.alias(table_name) if arel_table.name != table_name arel_table = arel_table.alias(table_name) if arel_table.name != table_name
TableMetadata.new(association_klass, arel_table, reflection) TableMetadata.new(association_klass, arel_table, reflection)
else else
type_caster = TypeCaster::Connection.new(klass, table_name) type_caster = TypeCaster::Connection.new(klass)
arel_table = Arel::Table.new(table_name, type_caster: type_caster) arel_table = Arel::Table.new(table_name, type_caster: type_caster)
TableMetadata.new(nil, arel_table, reflection) TableMetadata.new(nil, arel_table, reflection)
end end

View File

@ -393,8 +393,8 @@ module ActiveRecord
# This method is available within the context of an ActiveRecord::Base # This method is available within the context of an ActiveRecord::Base
# instance. # instance.
def with_transaction_returning_status def with_transaction_returning_status
self.class.with_connection do |connection|
status = nil status = nil
connection = self.class.lease_connection
ensure_finalize = !connection.transaction_open? ensure_finalize = !connection.transaction_open?
connection.transaction do connection.transaction do
@ -406,6 +406,7 @@ module ActiveRecord
end end
status status
end end
end
def trigger_transactional_callbacks? # :nodoc: def trigger_transactional_callbacks? # :nodoc:
(@_new_record_before_last_commit || _trigger_update_callback) && persisted? || (@_new_record_before_last_commit || _trigger_update_callback) && persisted? ||
@ -496,7 +497,9 @@ module ActiveRecord
# Add the record to the current transaction so that the #after_rollback and #after_commit # Add the record to the current transaction so that the #after_rollback and #after_commit
# callbacks can be called. # callbacks can be called.
def add_to_transaction(ensure_finalize = true) def add_to_transaction(ensure_finalize = true)
self.class.lease_connection.add_transaction_record(self, ensure_finalize) self.class.with_connection do |connection|
connection.add_transaction_record(self, ensure_finalize)
end
end end
def has_transactional_callbacks? def has_transactional_callbacks?

View File

@ -3,9 +3,8 @@
module ActiveRecord module ActiveRecord
module TypeCaster module TypeCaster
class Connection # :nodoc: class Connection # :nodoc:
def initialize(klass, table_name) def initialize(klass)
@klass = klass @klass = klass
@table_name = table_name
end end
def type_cast_for_database(attr_name, value) def type_cast_for_database(attr_name, value)
@ -14,20 +13,8 @@ module ActiveRecord
end end
def type_for_attribute(attr_name) def type_for_attribute(attr_name)
schema_cache = @klass.schema_cache @klass.type_for_attribute(attr_name) || Type.default_value
end
if schema_cache.data_source_exists?(table_name)
column = schema_cache.columns_hash(table_name)[attr_name.to_s]
if column
type = @klass.lease_connection.lookup_cast_type_from_column(column)
end
end
type || Type.default_value
end
private
attr_reader :table_name
end end
end end
end end

View File

@ -110,16 +110,20 @@ module ActiveRecord
def build_relation(klass, attribute, value) def build_relation(klass, attribute, value)
relation = klass.unscoped relation = klass.unscoped
comparison = relation.bind_attribute(attribute, value) do |attr, bind| # TODO: Add case-sensitive / case-insensitive operators to Arel
# to no longer need to checkout a connection here.
comparison = klass.with_connection do |connection|
relation.bind_attribute(attribute, value) do |attr, bind|
return relation.none! if bind.unboundable? return relation.none! if bind.unboundable?
if !options.key?(:case_sensitive) || bind.nil? if !options.key?(:case_sensitive) || bind.nil?
klass.lease_connection.default_uniqueness_comparison(attr, bind) connection.default_uniqueness_comparison(attr, bind)
elsif options[:case_sensitive] elsif options[:case_sensitive]
klass.lease_connection.case_sensitive_comparison(attr, bind) connection.case_sensitive_comparison(attr, bind)
else else
# will use SQL LOWER function before comparison, unless it detects a case insensitive collation # will use SQL LOWER function before comparison, unless it detects a case insensitive collation
klass.lease_connection.case_insensitive_comparison(attr, bind) connection.case_insensitive_comparison(attr, bind)
end
end end
end end

View File

@ -147,8 +147,9 @@ module Arel # :nodoc: all
# Maybe we should just use `Table.engine`? :'( # Maybe we should just use `Table.engine`? :'(
def to_sql(engine = Table.engine) def to_sql(engine = Table.engine)
collector = Arel::Collectors::SQLString.new collector = Arel::Collectors::SQLString.new
collector = engine.lease_connection.visitor.accept self, collector engine.with_connection do |connection|
collector.value connection.visitor.accept(self, collector).value
end
end end
def fetch_attribute def fetch_attribute

View File

@ -52,8 +52,9 @@ module Arel # :nodoc: all
def to_sql(engine = Table.engine) def to_sql(engine = Table.engine)
collector = Arel::Collectors::SQLString.new collector = Arel::Collectors::SQLString.new
collector = engine.lease_connection.visitor.accept @ast, collector engine.with_connection do |connection|
collector.value engine.lease_connection.visitor.accept(@ast, collector).value
end
end end
def initialize_copy(other) def initialize_copy(other)

View File

@ -129,6 +129,10 @@ module FakeRecord
@connection_pool = ConnectionPool.new @connection_pool = ConnectionPool.new
end end
def with_connection(...)
connection_pool.with_connection(...)
end
def lease_connection def lease_connection
connection_pool.lease_connection connection_pool.lease_connection
end end