Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer Arel attribute lookup to the model class #23457

Merged
merged 2 commits into from
Feb 3, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scop
column = klass.columns_hash[reflection.type.to_s]

binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name))
constraint = constraint.and table[reflection.type].eq(Arel::Nodes::BindParam.new)
constraint = constraint.and klass.arel_attribute(reflection.type, table).eq(Arel::Nodes::BindParam.new)
end

joins << table.create_join(table, table.create_on(constraint), join_type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def association_key_name
# This is overridden by HABTM as the condition should be on the foreign_key column in
# the join table
def association_key
table[association_key_name]
klass.arel_attribute(association_key_name, table)
end

# The name of the key on the model which declares the association
Expand Down
5 changes: 5 additions & 0 deletions activerecord/lib/active_record/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ def arel_engine # :nodoc:
end
end

def arel_attribute(name, table = arel_table) # :nodoc:
name = attribute_alias(name) if attribute_alias?(name)
table[name]
end

def predicate_builder # :nodoc:
@predicate_builder ||= PredicateBuilder.new(table_metadata)
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/inheritance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def find_sti_class(type_name)
end

def type_condition(table = arel_table)
sti_column = table[inheritance_column]
sti_column = arel_attribute(inheritance_column, table)
sti_names = ([self] + descendants).map(&:sti_name)

sti_column.in(sti_names)
Expand Down
12 changes: 8 additions & 4 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def insert(values) # :nodoc:

if !primary_key_value && connection.prefetch_primary_key?(klass.table_name)
primary_key_value = connection.next_sequence_value(klass.sequence_name)
values[klass.arel_table[klass.primary_key]] = primary_key_value
values[arel_attribute(klass.primary_key)] = primary_key_value
end
end

Expand Down Expand Up @@ -105,6 +105,10 @@ def substitute_values(values) # :nodoc:
[substitutes, binds]
end

def arel_attribute(name) # :nodoc:
klass.arel_attribute(name, table)
end

# Initializes new record from relation while maintaining the current
# scope.
#
Expand Down Expand Up @@ -373,9 +377,9 @@ def update_all(updates)
stmt.table(table)

if joins_values.any?
@klass.connection.join_to_update(stmt, arel, table[primary_key])
@klass.connection.join_to_update(stmt, arel, arel_attribute(primary_key))
else
stmt.key = table[primary_key]
stmt.key = arel_attribute(primary_key)
stmt.take(arel.limit)
stmt.order(*arel.orders)
stmt.wheres = arel.constraints
Expand Down Expand Up @@ -527,7 +531,7 @@ def delete_all(conditions = nil)
stmt.from(table)

if joins_values.any?
@klass.connection.join_to_delete(stmt, arel, table[primary_key])
@klass.connection.join_to_delete(stmt, arel, arel_attribute(primary_key))
else
stmt.wheres = arel.constraints
end
Expand Down
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,15 @@ def in_batches(of: 1000, start: nil, finish: nil, load: false)
yield yielded_relation

break if ids.length < of
batch_relation = relation.where(table[primary_key].gt(primary_key_offset))
batch_relation = relation.where(arel_attribute(primary_key).gt(primary_key_offset))
end
end

private

def apply_limits(relation, start, finish)
relation = relation.where(table[primary_key].gteq(start)) if start
relation = relation.where(table[primary_key].lteq(finish)) if finish
relation = relation.where(arel_attribute(primary_key).gteq(start)) if start
relation = relation.where(arel_attribute(primary_key).lteq(finish)) if finish
relation
end

Expand Down
12 changes: 2 additions & 10 deletions activerecord/lib/active_record/relation/calculations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,7 @@ def calculate(operation, column_name)
# See also #ids.
#
def pluck(*column_names)
column_names.map! do |column_name|
if column_name.is_a?(Symbol) && attribute_alias?(column_name)
attribute_alias(column_name)
else
column_name.to_s
end
end

if loaded? && (column_names - @klass.column_names).empty?
if loaded? && (column_names.map(&:to_s) - @klass.attribute_names - @klass.attribute_aliases.keys).empty?
return @records.pluck(*column_names)
end

Expand All @@ -172,7 +164,7 @@ def pluck(*column_names)
else
relation = spawn
relation.select_values = column_names.map { |cn|
columns_hash.key?(cn) ? arel_table[cn] : cn
@klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn
}
result = klass.connection.select_all(relation.arel, nil, bound_attributes)
result.cast_values(klass.attribute_types)
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def first!
def last(limit = nil)
if limit
if order_values.empty? && primary_key
order(arel_table[primary_key].desc).limit(limit).reverse
order(arel_attribute(primary_key).desc).limit(limit).reverse
else
to_a.last(limit)
end
Expand Down Expand Up @@ -514,7 +514,7 @@ def find_nth_with_limit(index, limit)
# TODO: once the offset argument is removed from find_nth,
# find_nth_with_limit_and_offset can be merged into this method
relation = if order_values.empty? && primary_key
order(arel_table[primary_key].asc)
order(arel_attribute(primary_key).asc)
else
self
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class PredicateBuilder
class RelationHandler # :nodoc:
def call(attribute, value)
if value.select_values.empty?
value = value.select(value.klass.arel_table[value.klass.primary_key])
value = value.select(value.arel_attribute(value.klass.primary_key))
end

attribute.in(value.arel)
Expand Down
12 changes: 5 additions & 7 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1093,8 +1093,8 @@ def build_select(arel)

def arel_columns(columns)
columns.map do |field|
if (Symbol === field || String === field) && columns_hash.key?(field.to_s) && !from_clause.value
arel_table[field]
if (Symbol === field || String === field) && (klass.has_attribute?(field) || klass.attribute_alias?(field)) && !from_clause.value
arel_attribute(field)
elsif Symbol === field
connection.quote_table_name(field.to_s)
else
Expand All @@ -1105,7 +1105,7 @@ def arel_columns(columns)

def reverse_sql_order(order_query)
if order_query.empty?
return [table[primary_key].desc] if primary_key
return [arel_attribute(primary_key).desc] if primary_key
raise IrreversibleOrderError,
"Relation has no current order and table has no primary key to be used as default order"
end
Expand Down Expand Up @@ -1170,12 +1170,10 @@ def preprocess_order_args(order_args)
order_args.map! do |arg|
case arg
when Symbol
arg = klass.attribute_alias(arg) if klass.attribute_alias?(arg)
table[arg].asc
arel_attribute(arg).asc
when Hash
arg.map { |field, dir|
field = klass.attribute_alias(field) if klass.attribute_alias?(field)
table[field].send(dir.downcase)
arel_attribute(field).send(dir.downcase)
}
else
arg
Expand Down
6 changes: 5 additions & 1 deletion activerecord/lib/active_record/table_metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ def resolve_column_aliases(hash)
end

def arel_attribute(column_name)
arel_table[column_name]
if klass
klass.arel_attribute(column_name, arel_table)
else
arel_table[column_name]
end
end

def type(column_name)
Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/cases/relation/mutation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ def sanitize_sql(sql)
def sanitize_sql_for_order(sql)
sql
end

def arel_attribute(name, table)
table[name]
end
end

def relation
Expand Down