Skip to content

Commit

Permalink
Ensure JoinAssociation uses aliased table name when multiple associat…
Browse files Browse the repository at this point in the history
…ions have hash conditions on the same table
  • Loading branch information
lifo committed Apr 20, 2009
1 parent 7ce0778 commit 489abfd
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 5 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/associations.rb
Expand Up @@ -2123,7 +2123,7 @@ def association_join
klass.send(:type_condition, aliased_table_name)] unless klass.descends_from_active_record?

[through_reflection, reflection].each do |ref|
join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions]))} " if ref && ref.options[:conditions]
join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions], aliased_table_name))} " if ref && ref.options[:conditions]
end

join
Expand Down
Expand Up @@ -169,8 +169,8 @@ def interpolate_sql(sql, record = nil)
end

# Forwards the call to the reflection class.
def sanitize_sql(sql)
@reflection.klass.send(:sanitize_sql, sql)
def sanitize_sql(sql, table_name = @reflection.klass.quoted_table_name)
@reflection.klass.send(:sanitize_sql, sql, table_name)
end

# Assigns the ID of the owner to the corresponding foreign key in +record+.
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/base.rb
Expand Up @@ -2231,12 +2231,12 @@ def class_name_of_active_record_descendant(klass) #:nodoc:
# ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'"
# { :name => "foo'bar", :group_id => 4 } returns "name='foo''bar' and group_id='4'"
# "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'"
def sanitize_sql_for_conditions(condition)
def sanitize_sql_for_conditions(condition, table_name = quoted_table_name)
return nil if condition.blank?

case condition
when Array; sanitize_sql_array(condition)
when Hash; sanitize_sql_hash_for_conditions(condition)
when Hash; sanitize_sql_hash_for_conditions(condition, table_name)
else condition
end
end
Expand Down
Expand Up @@ -29,6 +29,11 @@ def test_construct_finder_sql_applies_association_conditions
assert_match /INNER JOIN .?categories.? ON.*AND.*.?General.?.*TERMINATING_MARKER/, sql
end

def test_construct_finder_sql_applies_aliases_tables_on_association_conditions
result = Author.find(:all, :joins => [:thinking_posts, :welcome_posts])
assert_equal authors(:david), result.first
end

def test_construct_finder_sql_unpacks_nested_joins
sql = Author.send(:construct_finder_sql, :joins => {:posts => [[:comments]]})
assert_no_match /inner join.*inner join.*inner join/i, sql, "only two join clauses should be present"
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/author.rb
Expand Up @@ -25,6 +25,8 @@ def testing_proxy_target
has_many :comments_with_order_and_conditions, :through => :posts, :source => :comments, :order => 'comments.body', :conditions => "comments.body like 'Thank%'"
has_many :comments_with_include, :through => :posts, :source => :comments, :include => :post

has_many :thinking_posts, :class_name => 'Post', :conditions => { :title => 'So I was thinking' }
has_many :welcome_posts, :class_name => 'Post', :conditions => { :title => 'Welcome to the weblog' }

has_many :comments_desc, :through => :posts, :source => :comments, :order => 'comments.id DESC'
has_many :limited_comments, :through => :posts, :source => :comments, :limit => 1
Expand Down

1 comment on commit 489abfd

@puelocesar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I'm having a problem with this commit. I call merge_conditions from ActiveRecord::Base statically on several places on my application. Now with your commit I can't use it anymore, because merge_conditions calls sanitize_sql, that depends on a table name, so my application is crashing because table_name doesn't exist when you call ActiveRecord::Base.merge_conditions. Any tip on how to solve it?

Please sign in to comment.