Browse files

Merge pull request #11241 from neerajdotname/deprecated-counter-sql

Removed support for deprecated `counter_sql`
  • Loading branch information...
2 parents 173d4b9 + 4bf1ecd commit 3a923e034bd8273c5b852a98cd18c79a13425db7 @rafaelfranca rafaelfranca committed Jul 2, 2013
View
4 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
View
2 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
View
26 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
View
2 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
View
2 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
View
21 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
View
24 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
View
17 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'

0 comments on commit 3a923e0

Please sign in to comment.