Skip to content

Commit fa04810

Browse files
committed
Eliminate missed lease_connection calls
Followup: rails#51353 Fix: rails#51722 Not too sure what happened, but I somehow missed quite a number of `lease_connection` calls inside Active Record.
1 parent 793ff00 commit fa04810

File tree

18 files changed

+287
-228
lines changed

18 files changed

+287
-228
lines changed

activerecord/lib/active_record/associations/alias_tracker.rb

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,23 @@ module ActiveRecord
66
module Associations
77
# Keeps track of table aliases for ActiveRecord::Associations::JoinDependency
88
class AliasTracker # :nodoc:
9-
def self.create(connection, initial_table, joins, aliases = nil)
10-
if joins.empty?
11-
aliases ||= Hash.new(0)
12-
elsif aliases
13-
default_proc = aliases.default_proc || proc { 0 }
14-
aliases.default_proc = proc { |h, k|
15-
h[k] = initial_count_for(connection, k, joins) + default_proc.call(h, k)
16-
}
17-
else
18-
aliases = Hash.new { |h, k|
19-
h[k] = initial_count_for(connection, k, joins)
20-
}
9+
def self.create(pool, initial_table, joins, aliases = nil)
10+
pool.with_connection do |connection|
11+
if joins.empty?
12+
aliases ||= Hash.new(0)
13+
elsif aliases
14+
default_proc = aliases.default_proc || proc { 0 }
15+
aliases.default_proc = proc { |h, k|
16+
h[k] = initial_count_for(connection, k, joins) + default_proc.call(h, k)
17+
}
18+
else
19+
aliases = Hash.new { |h, k|
20+
h[k] = initial_count_for(connection, k, joins)
21+
}
22+
end
23+
aliases[initial_table] = 1
24+
new(connection.table_alias_length, aliases)
2125
end
22-
aliases[initial_table] = 1
23-
new(connection, aliases)
2426
end
2527

2628
def self.initial_count_for(connection, name, table_joins)
@@ -46,9 +48,9 @@ def self.initial_count_for(connection, name, table_joins)
4648
end
4749

4850
# table_joins is an array of arel joins which might conflict with the aliases we assign here
49-
def initialize(connection, aliases)
50-
@aliases = aliases
51-
@connection = connection
51+
def initialize(table_alias_length, aliases)
52+
@aliases = aliases
53+
@table_alias_length = table_alias_length
5254
end
5355

5456
def aliased_table_for(arel_table, table_name = nil)
@@ -60,7 +62,7 @@ def aliased_table_for(arel_table, table_name = nil)
6062
arel_table = arel_table.alias(table_name) if arel_table.name != table_name
6163
else
6264
# Otherwise, we need to use an alias
63-
aliased_name = @connection.table_alias_for(yield)
65+
aliased_name = table_alias_for(yield)
6466

6567
# Update the count
6668
count = aliases[aliased_name] += 1
@@ -76,8 +78,12 @@ def aliased_table_for(arel_table, table_name = nil)
7678
attr_reader :aliases
7779

7880
private
81+
def table_alias_for(table_name)
82+
table_name[0...@table_alias_length].tr(".", "_")
83+
end
84+
7985
def truncate(name)
80-
name.slice(0, @connection.table_alias_length - 2)
86+
name.slice(0, @table_alias_length - 2)
8187
end
8288
end
8389
end

activerecord/lib/active_record/attributes.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,10 @@ def define_attribute(
239239

240240
def _default_attributes # :nodoc:
241241
@default_attributes ||= begin
242-
attributes_hash = columns_hash.transform_values do |column|
243-
ActiveModel::Attribute.from_database(column.name, column.default, type_for_column(column))
242+
attributes_hash = with_connection do |connection|
243+
columns_hash.transform_values do |column|
244+
ActiveModel::Attribute.from_database(column.name, column.default, type_for_column(connection, column))
245+
end
244246
end
245247

246248
attribute_set = ActiveModel::AttributeSet.new(attributes_hash)
@@ -295,7 +297,7 @@ def resolve_type_name(name, **options)
295297
Type.lookup(name, **options, adapter: Type.adapter_name_from(self))
296298
end
297299

298-
def type_for_column(column)
300+
def type_for_column(connection, column)
299301
hook_attribute_type(column.name, super)
300302
end
301303
end

activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ def initialize(...)
9393
end
9494
end
9595

96-
def lease_connection(**)
97-
connection = super
96+
def checkout_and_verify(connection)
97+
super
9898
connection.query_cache ||= query_cache
9999
connection
100100
end
@@ -147,19 +147,19 @@ def clear_query_cache
147147
end
148148
end
149149

150+
def query_cache
151+
@thread_query_caches.compute_if_absent(ActiveSupport::IsolatedExecutionState.context) do
152+
Store.new(@query_cache_max_size)
153+
end
154+
end
155+
150156
private
151157
def prune_thread_cache
152158
dead_threads = @thread_query_caches.keys.reject(&:alive?)
153159
dead_threads.each do |dead_thread|
154160
@thread_query_caches.delete(dead_thread)
155161
end
156162
end
157-
158-
def query_cache
159-
@thread_query_caches.compute_if_absent(ActiveSupport::IsolatedExecutionState.context) do
160-
Store.new(@query_cache_max_size)
161-
end
162-
end
163163
end
164164

165165
attr_accessor :query_cache

activerecord/lib/active_record/core.rb

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,9 @@ def type_caster # :nodoc:
376376
TypeCaster::Map.new(self)
377377
end
378378

379-
def cached_find_by_statement(key, &block) # :nodoc:
380-
cache = @find_by_statement_cache[lease_connection.prepared_statements]
381-
cache.compute_if_absent(key) { StatementCache.create(lease_connection, &block) }
379+
def cached_find_by_statement(connection, key, &block) # :nodoc:
380+
cache = @find_by_statement_cache[connection.prepared_statements]
381+
cache.compute_if_absent(key) { StatementCache.create(connection, &block) }
382382
end
383383

384384
private
@@ -419,21 +419,23 @@ def table_metadata
419419
end
420420

421421
def cached_find_by(keys, values)
422-
statement = cached_find_by_statement(keys) { |params|
423-
wheres = keys.index_with do |key|
424-
if key.is_a?(Array)
425-
[key.map { params.bind }]
426-
else
427-
params.bind
422+
with_connection do |connection|
423+
statement = cached_find_by_statement(connection, keys) { |params|
424+
wheres = keys.index_with do |key|
425+
if key.is_a?(Array)
426+
[key.map { params.bind }]
427+
else
428+
params.bind
429+
end
428430
end
429-
end
430-
where(wheres).limit(1)
431-
}
431+
where(wheres).limit(1)
432+
}
432433

433-
begin
434-
statement.execute(values.flatten, lease_connection, allow_retry: true).first
435-
rescue TypeError
436-
raise ActiveRecord::StatementInvalid
434+
begin
435+
statement.execute(values.flatten, lease_connection, allow_retry: true).first
436+
rescue TypeError
437+
raise ActiveRecord::StatementInvalid
438+
end
437439
end
438440
end
439441
end

activerecord/lib/active_record/model_schema.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ def sequence_name
379379

380380
def reset_sequence_name # :nodoc:
381381
@explicit_sequence_name = false
382-
@sequence_name = lease_connection.default_sequence_name(table_name, primary_key)
382+
@sequence_name = with_connection { |c| c.default_sequence_name(table_name, primary_key) }
383383
end
384384

385385
# Sets the name of the sequence to use when generating ids to the given
@@ -404,13 +404,13 @@ def sequence_name=(value)
404404
# Determines if the primary key values should be selected from their
405405
# corresponding sequence before the insert statement.
406406
def prefetch_primary_key?
407-
lease_connection.prefetch_primary_key?(table_name)
407+
with_connection { |c| c.prefetch_primary_key?(table_name) }
408408
end
409409

410410
# Returns the next value that will be used as the primary key on
411411
# an insert statement.
412412
def next_sequence_value
413-
lease_connection.next_sequence_value(sequence_name)
413+
with_connection { |c| c.next_sequence_value(sequence_name) }
414414
end
415415

416416
# Indicates whether the table associated with this class exists
@@ -435,10 +435,10 @@ def columns
435435
@columns ||= columns_hash.values.freeze
436436
end
437437

438-
def _returning_columns_for_insert # :nodoc:
438+
def _returning_columns_for_insert(connection) # :nodoc:
439439
@_returning_columns_for_insert ||= begin
440440
auto_populated_columns = columns.filter_map do |c|
441-
c.name if lease_connection.return_value_after_insert?(c)
441+
c.name if connection.return_value_after_insert?(c)
442442
end
443443

444444
auto_populated_columns.empty? ? Array(primary_key) : auto_populated_columns
@@ -523,7 +523,7 @@ def content_columns
523523
# end
524524
# end
525525
def reset_column_information
526-
lease_connection.clear_cache!
526+
connection_pool.active_connection&.clear_cache!
527527
([self] + descendants).each(&:undefine_attribute_methods)
528528
schema_cache.clear_data_source_cache!(table_name)
529529

@@ -617,8 +617,8 @@ def compute_table_name
617617
end
618618
end
619619

620-
def type_for_column(column)
621-
type = lease_connection.lookup_cast_type_from_column(column)
620+
def type_for_column(connection, column)
621+
type = connection.lookup_cast_type_from_column(column)
622622

623623
if immutable_strings_by_default && type.respond_to?(:to_immutable_string)
624624
type = type.to_immutable_string

activerecord/lib/active_record/persistence.rb

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ def delete(id_or_array)
568568
delete_by(primary_key => id_or_array)
569569
end
570570

571-
def _insert_record(values, returning) # :nodoc:
571+
def _insert_record(connection, values, returning) # :nodoc:
572572
primary_key = self.primary_key
573573
primary_key_value = nil
574574

@@ -583,12 +583,12 @@ def _insert_record(values, returning) # :nodoc:
583583

584584
with_connection do |c|
585585
if values.empty?
586-
im.insert(c.empty_insert_statement_value(primary_key))
586+
im.insert(connection.empty_insert_statement_value(primary_key))
587587
else
588588
im.insert(values.transform_keys { |name| arel_table[name] })
589589
end
590590

591-
c.insert(
591+
connection.insert(
592592
im, "#{self} Create", primary_key || false, primary_key_value,
593593
returning: returning
594594
)
@@ -1255,16 +1255,19 @@ def _update_record(attribute_names = self.attribute_names)
12551255
def _create_record(attribute_names = self.attribute_names)
12561256
attribute_names = attributes_for_create(attribute_names)
12571257

1258-
returning_columns = self.class._returning_columns_for_insert
1258+
self.class.with_connection do |connection|
1259+
returning_columns = self.class._returning_columns_for_insert(connection)
12591260

1260-
returning_values = self.class._insert_record(
1261-
attributes_with_values(attribute_names),
1262-
returning_columns
1263-
)
1261+
returning_values = self.class._insert_record(
1262+
connection,
1263+
attributes_with_values(attribute_names),
1264+
returning_columns
1265+
)
12641266

1265-
returning_columns.zip(returning_values).each do |column, value|
1266-
_write_attribute(column, value) if !_read_attribute(column)
1267-
end if returning_values
1267+
returning_columns.zip(returning_values).each do |column, value|
1268+
_write_attribute(column, value) if !_read_attribute(column)
1269+
end if returning_values
1270+
end
12681271

12691272
@new_record = false
12701273
@previously_new_record = true

activerecord/lib/active_record/querying.rb

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,25 @@ module Querying
4848
# Note that building your own SQL query string from user input may expose your application to
4949
# injection attacks (https://guides.rubyonrails.org/security.html#sql-injection).
5050
def find_by_sql(sql, binds = [], preparable: nil, allow_retry: false, &block)
51-
_load_from_sql(_query_by_sql(sql, binds, preparable: preparable, allow_retry: allow_retry), &block)
51+
result = with_connection do |c|
52+
_query_by_sql(c, sql, binds, preparable: preparable, allow_retry: allow_retry)
53+
end
54+
_load_from_sql(result, &block)
5255
end
5356

5457
# Same as <tt>#find_by_sql</tt> but perform the query asynchronously and returns an ActiveRecord::Promise.
5558
def async_find_by_sql(sql, binds = [], preparable: nil, &block)
56-
_query_by_sql(sql, binds, preparable: preparable, async: true).then do |result|
59+
result = with_connection do |c|
60+
_query_by_sql(c, sql, binds, preparable: preparable, async: true)
61+
end
62+
63+
result.then do |result|
5764
_load_from_sql(result, &block)
5865
end
5966
end
6067

61-
def _query_by_sql(sql, binds = [], preparable: nil, async: false, allow_retry: false) # :nodoc:
62-
lease_connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable, async: async, allow_retry: allow_retry)
68+
def _query_by_sql(connection, sql, binds = [], preparable: nil, async: false, allow_retry: false) # :nodoc:
69+
connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable, async: async, allow_retry: allow_retry)
6370
end
6471

6572
def _load_from_sql(result_set, &block) # :nodoc:
@@ -99,12 +106,16 @@ def _load_from_sql(result_set, &block) # :nodoc:
99106
#
100107
# * +sql+ - An SQL statement which should return a count query from the database, see the example above.
101108
def count_by_sql(sql)
102-
lease_connection.select_value(sanitize_sql(sql), "#{name} Count").to_i
109+
with_connection do |c|
110+
c.select_value(sanitize_sql(sql), "#{name} Count").to_i
111+
end
103112
end
104113

105114
# Same as <tt>#count_by_sql</tt> but perform the query asynchronously and returns an ActiveRecord::Promise.
106115
def async_count_by_sql(sql)
107-
lease_connection.select_value(sanitize_sql(sql), "#{name} Count", async: true).then(&:to_i)
116+
with_connection do |c|
117+
c.select_value(sanitize_sql(sql), "#{name} Count", async: true).then(&:to_i)
118+
end
108119
end
109120
end
110121
end

activerecord/lib/active_record/reflection.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,9 @@ def association_scope_cache(klass, owner, &block)
529529
if polymorphic?
530530
key = [key, owner._read_attribute(@foreign_type)]
531531
end
532-
klass.cached_find_by_statement(key, &block)
532+
klass.with_connection do |connection|
533+
klass.cached_find_by_statement(connection, key, &block)
534+
end
533535
end
534536

535537
def join_table

0 commit comments

Comments
 (0)