Removed support for deprecated counter_sql

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Removed support for deprecated `counter_sql` in associations.
+
+    *Neeraj Singh*
+
 *   Do not invoke callbacks when `delete_all` is called on collection.
 
     Method `delete_all` should not be invoking callbacks and this

activerecord/lib/active_record/associations/builder/collection_association.rb

@@ -8,7 +8,7 @@ class CollectionAssociation < Association #:nodoc:
     CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove]
 
     def valid_options
-      super + [:table_name, :finder_sql, :counter_sql, :before_add,
+      super + [:table_name, :finder_sql, :before_add,
                :after_add, :before_remove, :after_remove, :extend]
     end
 

activerecord/lib/active_record/associations/collection_association.rb

@@ -197,15 +197,15 @@ def destroy_all
         end
       end
 
-      # Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the
+      # Count all records using SQL. If the +:finder_sql+ option is set for the
       # association, it will be used for the query. Otherwise, construct options and pass them with
       # scope to the target class's +count+.
       def count(column_name = nil, count_options = {})
         column_name, count_options = nil, column_name if column_name.is_a?(Hash)
 
-        if options[:counter_sql] || options[:finder_sql]
+        if options[:finder_sql]
           unless count_options.blank?
-            raise ArgumentError, "If finder_sql/counter_sql is used then options cannot be passed"
+            raise ArgumentError, "If finder_sql is used then options cannot be passed"
           end
 
           reflection.klass.count_by_sql(custom_counter_sql)
@@ -301,14 +301,14 @@ def length
 
       # Returns true if the collection is empty.
       #
-      # If the collection has been loaded or the <tt>:counter_sql</tt> option
-      # is provided, it is equivalent to <tt>collection.size.zero?</tt>. If the
+      # If the collection has been loaded 
+      # it is equivalent to <tt>collection.size.zero?</tt>. If the
       # collection has not been loaded, it is equivalent to
       # <tt>collection.exists?</tt>. If the collection has not already been
       # loaded and you are going to fetch the records anyway it is better to
       # check <tt>collection.length.zero?</tt>.
       def empty?
-        if loaded? || options[:counter_sql]
+        if loaded?
           size.zero?
         else
           @target.blank? && !scope.exists?
@@ -407,15 +407,11 @@ def null_scope?
       private
 
         def custom_counter_sql
-          if options[:counter_sql]
-            interpolate(options[:counter_sql])
-          else
-            # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */
-            interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do
-              count_with = $2.to_s
-              count_with = '*' if count_with.blank? || count_with =~ /,/ || count_with =~ /\.\*/
-              "SELECT #{$1}COUNT(#{count_with}) FROM"
-            end
+          # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */
+          interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do
+            count_with = $2.to_s
+            count_with = '*' if count_with.blank? || count_with =~ /,/ || count_with =~ /\.\*/
+            "SELECT #{$1}COUNT(#{count_with}) FROM"
           end
         end
 

activerecord/lib/active_record/associations/collection_proxy.rb

@@ -726,7 +726,7 @@ def length
       end
 
       # Returns +true+ if the collection is empty. If the collection has been
-      # loaded or the <tt>:counter_sql</tt> option is provided, it is equivalent
+      # loaded it is equivalent
       # to <tt>collection.size.zero?</tt>. If the collection has not been loaded,
       # it is equivalent to <tt>collection.exists?</tt>. If the collection has
       # not already been loaded and you are going to fetch the records anyway it

activerecord/lib/active_record/associations/has_many_association.rb

@@ -58,7 +58,7 @@ def insert_record(record, validate = true, raise = false)
         def count_records
           count = if has_cached_counter?
             owner.send(:read_attribute, cached_counter_attribute_name)
-          elsif options[:counter_sql] || options[:finder_sql]
+          elsif options[:finder_sql]
             reflection.klass.count_by_sql(custom_counter_sql)
           else
             scope.count

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

@@ -65,19 +65,6 @@ class DeveloperWithSymbolsForKeys < ActiveRecord::Base
     :foreign_key => "developer_id"
 end
 
-class DeveloperWithCounterSQL < ActiveRecord::Base
-  self.table_name = 'developers'
-
-  ActiveSupport::Deprecation.silence do
-    has_and_belongs_to_many :projects,
-      :class_name => "DeveloperWithCounterSQL",
-      :join_table => "developers_projects",
-      :association_foreign_key => "project_id",
-      :foreign_key => "developer_id",
-      :counter_sql => proc { "SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}" }
-  end
-end
-
 class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects,
            :parrots, :pirates, :parrots_pirates, :treasures, :price_estimates, :tags, :taggings
@@ -791,14 +778,6 @@ def test_count
     assert_equal 2, david.projects.count
   end
 
-  def test_count_with_counter_sql
-    developer  = DeveloperWithCounterSQL.create(:name => 'tekin')
-    developer.project_ids = [projects(:active_record).id]
-    developer.save
-    developer.reload
-    assert_equal 1, developer.projects.count
-  end
-
   unless current_adapter?(:PostgreSQLAdapter)
     def test_count_with_finder_sql
       assert_equal 3, projects(:active_record).developers_with_finder_sql.count

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -36,19 +36,6 @@ def test_should_fail
   end
 end
 
-class HasManyAssociationsTestForCountWithCountSql < ActiveRecord::TestCase
-  class Invoice < ActiveRecord::Base
-    ActiveSupport::Deprecation.silence do
-      has_many :custom_line_items, :class_name => 'LineItem', :counter_sql => "SELECT COUNT(*) line_items.* from line_items"
-    end
-  end
-  def test_should_fail
-    assert_raise(ArgumentError) do
-      Invoice.create.custom_line_items.count(:conditions => {:amount => 0})
-    end
-  end
-end
-
 class HasManyAssociationsTestForCountWithVariousFinderSqls < ActiveRecord::TestCase
   class Invoice < ActiveRecord::Base
     ActiveSupport::Deprecation.silence do
@@ -381,17 +368,6 @@ def test_finding_using_sql_take_into_account_only_uniq_ids
     assert_equal client, firm.clients_using_sql.find(client.id, client.id.to_s)
   end
 
-  def test_counting_using_sql
-    assert_equal 1, Firm.order("id").first.clients_using_counter_sql.size
-    assert Firm.order("id").first.clients_using_counter_sql.any?
-    assert_equal 0, Firm.order("id").first.clients_using_zero_counter_sql.size
-    assert !Firm.order("id").first.clients_using_zero_counter_sql.any?
-  end
-
-  def test_counting_non_existant_items_using_sql
-    assert_equal 0, Firm.order("id").first.no_clients_using_counter_sql.size
-  end
-
   def test_counting_using_finder_sql
     assert_equal 2, Firm.find(4).clients_using_sql.count
   end

activerecord/test/models/company.rb

@@ -35,13 +35,7 @@ class Client < ::Company
 end
 
 class Firm < Company
-  ActiveSupport::Deprecation.silence do
-    has_many :clients, -> { order "id" }, :dependent => :destroy, :counter_sql =>
-        "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " +
-        "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )",
-        :before_remove => :log_before_remove,
-        :after_remove  => :log_after_remove
-  end
+  has_many :clients, -> { order "id" }, :dependent => :destroy, :before_remove => :log_before_remove, :after_remove  => :log_after_remove
   has_many :unsorted_clients, :class_name => "Client"
   has_many :unsorted_clients_with_symbol, :class_name => :Client
   has_many :clients_sorted_desc, -> { order "id DESC" }, :class_name => "Client"
@@ -56,15 +50,6 @@ class Firm < Company
   has_many :clients_like_ms_with_hash_conditions, -> { where(:name => 'Microsoft').order("id") }, :class_name => "Client"
   ActiveSupport::Deprecation.silence do
     has_many :clients_using_sql, :class_name => "Client", :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" }
-    has_many :clients_using_counter_sql, :class_name => "Client",
-             :finder_sql  => proc { "SELECT * FROM companies WHERE client_of = #{id} " },
-             :counter_sql => proc { "SELECT COUNT(*) FROM companies WHERE client_of = #{id}" }
-    has_many :clients_using_zero_counter_sql, :class_name => "Client",
-             :finder_sql  => proc { "SELECT * FROM companies WHERE client_of = #{id}" },
-             :counter_sql => proc { "SELECT 0 FROM companies WHERE client_of = #{id}" }
-    has_many :no_clients_using_counter_sql, :class_name => "Client",
-             :finder_sql  => 'SELECT * FROM companies WHERE client_of = 1000',
-             :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000'
     has_many :clients_using_finder_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE 1=1'
   end
   has_many :plain_clients, :class_name => 'Client'