diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 84edaec15e047..e12f6be35dc9c 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -351,19 +351,7 @@ def proxy_respond_to?(method, include_private = false) protected def construct_find_options!(options) end - - def construct_counter_sql - if @reflection.options[:counter_sql] - @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) - elsif @reflection.options[:finder_sql] - # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ - @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } - @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) - else - @counter_sql = @finder_sql - end - end - + def load_target if !@owner.new_record? || foreign_key_present begin diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index fd23e59e82590..af9ce3dfb2534 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -85,7 +85,15 @@ def construct_sql @join_sql = "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}" - construct_counter_sql + if @reflection.options[:counter_sql] + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + elsif @reflection.options[:finder_sql] + # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ + @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + else + @counter_sql = @finder_sql + end end def construct_scope diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index e4b631bc54d92..73dd50dd0747d 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -97,7 +97,15 @@ def construct_sql @finder_sql << " AND (#{conditions})" if conditions end - construct_counter_sql + if @reflection.options[:counter_sql] + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + elsif @reflection.options[:finder_sql] + # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ + @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + else + @counter_sql = @finder_sql + end end def construct_scope diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index e21ef9039101a..67631f0120517 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -90,7 +90,15 @@ def construct_sql @finder_sql = construct_conditions end - construct_counter_sql + if @reflection.options[:counter_sql] + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + elsif @reflection.options[:finder_sql] + # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ + @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + else + @counter_sql = @finder_sql + end end def has_cached_counter? diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 9dbbd01aa1d25..8dc95806b940a 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -803,11 +803,6 @@ def test_count_with_counter_sql assert_equal 1, developer.projects.count end - def test_count_with_finder_sql - assert_equal 3, projects(:active_record).developers_with_finder_sql.count - assert_equal 3, projects(:active_record).developers_with_multiline_finder_sql.count - end - def test_association_proxy_transaction_method_starts_transaction_in_association_class Post.expects(:transaction) Category.find(:first).posts.transaction do diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 15919e228957f..d99424f9cd060 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -163,11 +163,6 @@ def test_counting_non_existant_items_using_sql assert_equal 0, Firm.find(:first).no_clients_using_counter_sql.size end - def test_counting_using_finder_sql - assert_equal 2, Firm.find(4).clients_using_sql.count - assert_equal 2, Firm.find(4).clients_using_multiline_sql.count - end - def test_belongs_to_sanity c = Client.new assert_nil c.firm diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 0f3f6e41a54bd..96270118d8279 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -599,9 +599,9 @@ def test_destroy_all end def test_destroy_many - assert_difference('Client.count', -2) do - Client.destroy([2, 3]) - end + assert_equal 3, Client.count + Client.destroy([2, 3]) + assert_equal 1, Client.count end def test_delete_many diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 75f52dfa4ae00..fd455e0864e80 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -203,7 +203,7 @@ def test_should_calculate_grouped_by_function c = Company.count(:all, :group => "UPPER(#{QUOTED_TYPE})") assert_equal 2, c[nil] assert_equal 1, c['DEPENDENTFIRM'] - assert_equal 4, c['CLIENT'] + assert_equal 3, c['CLIENT'] assert_equal 2, c['FIRM'] end @@ -211,7 +211,7 @@ def test_should_calculate_grouped_by_function_with_table_alias c = Company.count(:all, :group => "UPPER(companies.#{QUOTED_TYPE})") assert_equal 2, c[nil] assert_equal 1, c['DEPENDENTFIRM'] - assert_equal 4, c['CLIENT'] + assert_equal 3, c['CLIENT'] assert_equal 2, c['FIRM'] end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index d8f5695a0fe8c..037b67ec8441e 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1044,8 +1044,8 @@ def test_select_value end def test_select_values - assert_equal ["1","2","3","4","5","6","7","8","9", "10"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map! { |i| i.to_s } - assert_equal ["37signals","Summit","Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy", "Ex Nihilo Part Deux"], Company.connection.select_values("SELECT name FROM companies ORDER BY id") + assert_equal ["1","2","3","4","5","6","7","8","9"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map! { |i| i.to_s } + assert_equal ["37signals","Summit","Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy"], Company.connection.select_values("SELECT name FROM companies ORDER BY id") end def test_select_rows diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 167d3abad91f3..eae5a60829182 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -112,9 +112,9 @@ def test_alt_inheritance_save end def test_inheritance_condition - assert_equal 10, Company.count + assert_equal 9, Company.count assert_equal 2, Firm.count - assert_equal 4, Client.count + assert_equal 3, Client.count end def test_alt_inheritance_condition diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 9275c9af89d0f..db64bbb8062e5 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -170,8 +170,8 @@ def test_association_reflection_in_modules def test_reflection_of_all_associations # FIXME these assertions bust a lot - assert_equal 29, Firm.reflect_on_all_associations.size - assert_equal 22, Firm.reflect_on_all_associations(:has_many).size + assert_equal 28, Firm.reflect_on_all_associations.size + assert_equal 21, Firm.reflect_on_all_associations(:has_many).size assert_equal 7, Firm.reflect_on_all_associations(:has_one).size assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end diff --git a/activerecord/test/fixtures/companies.yml b/activerecord/test/fixtures/companies.yml index 9ad68fbe116a1..e7691fde46224 100644 --- a/activerecord/test/fixtures/companies.yml +++ b/activerecord/test/fixtures/companies.yml @@ -35,14 +35,6 @@ another_client: name: Ex Nihilo ruby_type: Client -a_third_client: - id: 10 - type: Client - firm_id: 4 - client_of: 4 - name: Ex Nihilo Part Deux - ruby_type: Client - rails_core: id: 6 name: RailsCore diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 840527ddeb9bc..4c2514b0b3cdb 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -48,10 +48,6 @@ class Firm < Company has_many :clients_with_interpolated_conditions, :class_name => "Client", :conditions => 'rating > #{rating}' has_many :clients_like_ms_with_hash_conditions, :conditions => { :name => 'Microsoft' }, :class_name => "Client", :order => "id" has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' - has_many :clients_using_multiline_sql, :class_name => "Client", :finder_sql => ' - SELECT - companies.* - FROM companies WHERE companies.client_of = #{id}' has_many :clients_using_counter_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}', :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = #{id}' diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb index 422b12dc8310f..f25b2ddf77cc6 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -8,12 +8,6 @@ class Project < ActiveRecord::Base has_and_belongs_to_many :developers_named_david_with_hash_conditions, :class_name => "Developer", :conditions => { :name => 'David' }, :uniq => true has_and_belongs_to_many :salaried_developers, :class_name => "Developer", :conditions => "salary > 0" has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => 'SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id' - has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => ' - SELECT - t.*, j.* - FROM - developers_projects j, - developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id' has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => "DELETE FROM developers_projects WHERE project_id = \#{id} AND developer_id = \#{record.id}" has_and_belongs_to_many :developers_with_callbacks, :class_name => "Developer", :before_add => Proc.new {|o, r| o.developers_log << "before_adding#{r.id || ''}"}, :after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || ''}"},