Skip to content

Commit

Permalink
Ensure column names on reflection as a string
Browse files Browse the repository at this point in the history
If association column names are defined as a symbol, association access
will cause `Symbol#to_s` each time since attributes and columns hash
keys are a string.

To avoid that extra `Symbol#to_s` allocation, ensure column names on
reflection as a string.
  • Loading branch information
kamipo committed Jun 2, 2020
1 parent a7c1873 commit c0750fe
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 19 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/associations/association.rb
Expand Up @@ -315,7 +315,7 @@ def invertible_for?(record)

# Returns true if record contains the foreign_key
def foreign_key_for?(record)
record._has_attribute?(reflection.foreign_key.to_s)
record._has_attribute?(reflection.foreign_key)
end

# This should be implemented to return the values of the relevant key(s) on the owner,
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -470,7 +470,7 @@ def record_changed?(reflection, record, key)
def association_foreign_key_changed?(reflection, record, key)
return false if reflection.through_reflection?

record._has_attribute?(reflection.foreign_key.to_s) && record._read_attribute(reflection.foreign_key) != key
record._has_attribute?(reflection.foreign_key) && record._read_attribute(reflection.foreign_key) != key
end

# Saves the associated record if it's new or <tt>:autosave</tt> is enabled.
Expand Down
34 changes: 21 additions & 13 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -162,7 +162,7 @@ def build_association(attributes, &block)
# <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>'Money'</tt>
# <tt>has_many :clients</tt> returns <tt>'Client'</tt>
def class_name
@class_name ||= (options[:class_name] || derive_class_name).to_s
@class_name ||= -(options[:class_name]&.to_s || derive_class_name)
end

JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
Expand Down Expand Up @@ -218,14 +218,14 @@ def constraints
end

def counter_cache_column
if belongs_to?
@counter_cache_column ||= if belongs_to?
if options[:counter_cache] == true
"#{active_record.name.demodulize.underscore.pluralize}_count"
-"#{active_record.name.demodulize.underscore.pluralize}_count"
elsif options[:counter_cache]
options[:counter_cache].to_s
-options[:counter_cache].to_s
end
else
options[:counter_cache] ? options[:counter_cache].to_s : "#{name}_count"
-(options[:counter_cache]&.to_s || "#{name}_count")
end
end

Expand Down Expand Up @@ -432,8 +432,8 @@ def compute_class(name)

def initialize(name, scope, options, active_record)
super
@type = options[:as] && (options[:foreign_type] || "#{options[:as]}_type")
@foreign_type = options[:polymorphic] && (options[:foreign_type] || "#{name}_type")
@type = -(options[:foreign_type]&.to_s || "#{options[:as]}_type") if options[:as]
@foreign_type = -(options[:foreign_type]&.to_s || "#{name}_type") if options[:polymorphic]
@constructable = calculate_constructable(macro, options)

if options[:class_name] && options[:class_name].class == Class
Expand All @@ -454,24 +454,28 @@ def constructable? # :nodoc:
end

def join_table
@join_table ||= options[:join_table] || derive_join_table
@join_table ||= -(options[:join_table]&.to_s || derive_join_table)
end

def foreign_key
@foreign_key ||= options[:foreign_key] || derive_foreign_key.freeze
@foreign_key ||= -(options[:foreign_key]&.to_s || derive_foreign_key)
end

def association_foreign_key
@association_foreign_key ||= options[:association_foreign_key] || class_name.foreign_key
@association_foreign_key ||= -(options[:association_foreign_key]&.to_s || class_name.foreign_key)
end

# klass option is necessary to support loading polymorphic associations
def association_primary_key(klass = nil)
options[:primary_key] || primary_key(klass || self.klass)
if primary_key = options[:primary_key]
@association_primary_key ||= -primary_key.to_s
else
primary_key(klass || self.klass)
end
end

def active_record_primary_key
@active_record_primary_key ||= options[:primary_key] || primary_key(active_record)
@active_record_primary_key ||= -(options[:primary_key]&.to_s || primary_key(active_record))
end

def check_validity!
Expand Down Expand Up @@ -872,7 +876,11 @@ def nested?
def association_primary_key(klass = nil)
# Get the "actual" source reflection if the immediate source reflection has a
# source reflection itself
actual_source_reflection.options[:primary_key] || primary_key(klass || self.klass)
if primary_key = actual_source_reflection.options[:primary_key]
@association_primary_key ||= -primary_key.to_s
else
primary_key(klass || self.klass)
end
end

# Gets an array of possible <tt>:through</tt> source reflection names in both singular and plural form.
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation.rb
Expand Up @@ -308,7 +308,7 @@ def many?
# last updated record.
#
# Product.where("name like ?", "%Game%").cache_key(:last_reviewed_at)
def cache_key(timestamp_column = :updated_at)
def cache_key(timestamp_column = "updated_at")
@cache_keys ||= {}
@cache_keys[timestamp_column] ||= klass.collection_cache_key(self, timestamp_column)
end
Expand Down
Expand Up @@ -9,7 +9,7 @@ def initialize(associated_table, value)
end

def queries
[associated_table.association_join_foreign_key.to_s => ids]
[associated_table.association_join_foreign_key => ids]
end

private
Expand Down
Expand Up @@ -11,8 +11,8 @@ def initialize(associated_table, values)
def queries
type_to_ids_mapping.map do |type, ids|
{
associated_table.association_foreign_type.to_s => type,
associated_table.association_foreign_key.to_s => ids
associated_table.association_foreign_type => type,
associated_table.association_foreign_key => ids
}
end
end
Expand Down

0 comments on commit c0750fe

Please sign in to comment.