Skip to content

Commit

Permalink
Specify the STI type condition using SQL IN rather than a whole load …
Browse files Browse the repository at this point in the history
…of ORs. Required a fix to ActiveRecord::Relation#merge for properly merging create_with_value. This also fixes a situation where the type condition was appearing twice in the resultant SQL query.
  • Loading branch information
jonleighton committed Dec 31, 2010
1 parent 2bf3186 commit 62b084f
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def create_record(attrs, &block)
ensure_owner_is_persisted!

transaction do
with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do
with_scope(:create => @scope[:create].merge(scoped.scope_for_create)) do
build_record(attrs, &block)
end
end
Expand Down
12 changes: 8 additions & 4 deletions activerecord/lib/active_record/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,12 @@ def before_remove_const #:nodoc:

def relation #:nodoc:
@relation ||= Relation.new(self, arel_table)
finder_needs_type_condition? ? @relation.where(type_condition) : @relation

if finder_needs_type_condition?
@relation.where(type_condition).create_with(inheritance_column.to_sym => sti_name)
else
@relation
end
end

# Finder methods must instantiate through this method to work with the
Expand Down Expand Up @@ -914,10 +919,9 @@ def construct_finder_arel(options = {}, scope = nil)

def type_condition
sti_column = arel_table[inheritance_column.to_sym]
condition = sti_column.eq(sti_name)
descendants.each { |subclass| condition = condition.or(sti_column.eq(subclass.sti_name)) }
sti_names = ([self] + descendants).map { |model| model.sti_name }

condition
sti_column.in(sti_names)
end

# Guesses the table name, but does not decorate it with prefix and suffix information.
Expand Down
6 changes: 5 additions & 1 deletion activerecord/lib/active_record/relation/spawn_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,17 @@ def merge(r)

merged_relation.where_values = merged_wheres

(Relation::SINGLE_VALUE_METHODS - [:lock]).each do |method|
(Relation::SINGLE_VALUE_METHODS - [:lock, :create_with]).each do |method|
value = r.send(:"#{method}_value")
merged_relation.send(:"#{method}_value=", value) unless value.nil?
end

merged_relation.lock_value = r.lock_value unless merged_relation.lock_value

if r.create_with_value
merged_relation.create_with_value = (merged_relation.create_with_value || {}).merge(r.create_with_value)
end

# Apply scope extension modules
merged_relation.send :apply_modules, r.extensions

Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/cases/relation_scoping_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -485,4 +485,11 @@ def test_scope_composed_by_limit_and_then_offset_is_equal_to_scope_composed_by_o
posts_offset_limit = Post.offset(2).limit(3)
assert_equal posts_limit_offset, posts_offset_limit
end

def test_create_with_merge
aaron = (PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20) &
PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new
assert_equal 20, aaron.salary
assert_equal 'Aaron', aaron.name
end
end

0 comments on commit 62b084f

Please sign in to comment.