Merge pull request rails#51453 from fatkodima/active-counter-caches

Add the ability to ignore counter cache columns while they are backfilling

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,27 @@
+*   Add the ability to ignore counter cache columns until they are backfilled
+
+    Starting to use counter caches on existing large tables can be troublesome, because the column
+    values must be backfilled separately of the column addition (to not lock the table for too long)
+    and before the use of `:counter_cache` (otherwise methods like `size`/`any?`/etc, which use
+    counter caches internally, can produce incorrect results). People usually use database triggers
+    or callbacks on child associations while backfilling before introducing a counter cache
+    configuration to the association.
+
+    Now, to safely backfill the column, while keeping the column updated with child records added/removed, use:
+
+    ```ruby
+    class Comment < ApplicationRecord
+      belongs_to :post, counter_cache: { active: false }
+    end
+    ```
+
+    While the counter cache is not "active", the methods like `size`/`any?`/etc will not use it,
+    but get the results directly from the database. After the counter cache column is backfilled, simply
+    remove the `{ active: false }` part from the counter cache definition, and it will now be used by the
+    mentioned methods.
+
+    *fatkodima*
+
 *   Retry known idempotent SELECT queries on connection-related exceptions
 
     SELECT queries we construct by walking the Arel tree and / or with known model attributes

activerecord/lib/active_record/associations.rb

@@ -1819,6 +1819,16 @@ def has_one(name, scope = nil, **options)
         #   return the count cached, see note below). You can also specify a custom counter
         #   cache column by providing a column name instead of a +true+/+false+ value to this
         #   option (e.g., <tt>counter_cache: :my_custom_counter</tt>.)
+        #
+        #   Starting to use counter caches on existing large tables can be troublesome, because the column
+        #   values must be backfilled separately of the column addition (to not lock the table for too long)
+        #   and before the use of +:counter_cache+ (otherwise methods like +size+/+any?+/etc, which use
+        #   counter caches internally, can produce incorrect results). To safely backfill the values while keeping
+        #   counter cache columns updated with the child records creation/removal and to avoid the mentioned methods
+        #   use the possibly incorrect counter cache column values and always get the results from the database,
+        #   use <tt>counter_cache: { active: false }</tt>.
+        #   If you also need to specify a custom column name, use <tt>counter_cache: { active: false, column: :my_custom_counter }</tt>.
+        #
         #   Note: Specifying a counter cache will add it to that model's list of readonly attributes
         #   using +attr_readonly+.
         # [+:polymorphic+]

activerecord/lib/active_record/associations/collection_association.rb

@@ -228,7 +228,7 @@ def size
       # loaded and you are going to fetch the records anyway it is better to
       # check <tt>collection.length.zero?</tt>.
       def empty?
-        if loaded? || @association_ids || reflection.has_cached_counter?
+        if loaded? || @association_ids || reflection.has_active_cached_counter?
           size.zero?
         else
           target.empty? && !scope.exists?

activerecord/lib/active_record/associations/has_many_association.rb

@@ -78,7 +78,7 @@ def insert_record(record, validate = true, raise = false)
         # If the collection is empty the target is set to an empty array and
         # the loaded flag is set to true as well.
         def count_records
-          count = if reflection.has_cached_counter?
+          count = if reflection.has_active_cached_counter?
             owner.read_attribute(reflection.counter_cache_column).to_i
           else
             scope.count(:all)

activerecord/lib/active_record/reflection.rb

@@ -236,14 +236,16 @@ def constraints
       end
 
       def counter_cache_column
-        @counter_cache_column ||= if belongs_to?
-          if options[:counter_cache] == true
-            -"#{active_record.name.demodulize.underscore.pluralize}_count"
-          elsif options[:counter_cache]
-            -options[:counter_cache].to_s
+        @counter_cache_column ||= begin
+          counter_cache = options[:counter_cache]
+
+          if belongs_to?
+            if counter_cache
+              counter_cache[:column] || -"#{active_record.name.demodulize.underscore.pluralize}_count"
+            end
+          else
+            -((counter_cache && -counter_cache[:column]) || "#{name}_count")
           end
-        else
-          -(options[:counter_cache]&.to_s || "#{name}_count")
         end
       end
 
@@ -292,7 +294,7 @@ def inverse_updates_counter_in_memory?
         inverse_of && inverse_which_updates_counter_cache == inverse_of
       end
 
-      # Returns whether a counter cache should be used for this association.
+      # Returns whether this association has a counter cache.
       #
       # The counter_cache option must be given on either the owner or inverse
       # association, and the column must be present on the owner.
@@ -302,6 +304,17 @@ def has_cached_counter?
           active_record.has_attribute?(counter_cache_column)
       end
 
+      # Returns whether this association has a counter cache and its column values were backfilled
+      # (and so it is used internally by methods like +size+/+any?+/etc).
+      def has_active_cached_counter?
+        return false unless has_cached_counter?
+
+        counter_cache = options[:counter_cache] ||
+                        (inverse_which_updates_counter_cache && inverse_which_updates_counter_cache.options[:counter_cache])
+
+        counter_cache[:active] != false
+      end
+
       def counter_must_be_updated_by_has_many?
         !inverse_updates_counter_in_memory? && has_cached_counter?
       end
@@ -378,7 +391,7 @@ def initialize(name, scope, options, active_record)
         super()
         @name          = name
         @scope         = scope
-        @options       = options
+        @options       = normalize_options(options)
         @active_record = active_record
         @klass         = options[:anonymous_class]
         @plural_name   = active_record.pluralize_table_names ?
@@ -434,6 +447,26 @@ def scope_for(relation, owner = nil)
         def derive_class_name
           name.to_s.camelize
         end
+
+        def normalize_options(options)
+          counter_cache = options.delete(:counter_cache)
+
+          if counter_cache
+            active = true
+
+            case counter_cache
+            when String, Symbol
+              column = -counter_cache.to_s
+            when Hash
+              active = counter_cache.fetch(:active, true)
+              column = counter_cache[:column]&.to_s
+            end
+
+            options[:counter_cache] = { active: active, column: column }
+          end
+
+          options
+        end
     end
 
     # Holds all the metadata about an aggregation as it was specified in the

activerecord/test/cases/counter_cache_test.rb

@@ -8,6 +8,7 @@
 require "models/aircraft"
 require "models/wheel"
 require "models/engine"
+require "models/tyre"
 require "models/reply"
 require "models/category"
 require "models/categorization"
@@ -443,6 +444,43 @@ class ::SpecialReply < ::Reply
     assert_not Car.counter_cache_column?("cars_count")
   end
 
+  test "inactive conter cache" do
+    car = Car.new
+    car.bulbs = [Bulb.new, Bulb.new]
+    car.save!
+
+    assert_equal 2, car.bulbs_count
+    car.reload
+
+    assert_queries_count(5) do
+      assert_equal 2, car.bulbs.size
+      assert_equal 2, car.bulbs.count
+      assert_not_predicate car.bulbs, :empty?
+      assert_predicate car.bulbs, :any?
+      assert_not_predicate car.bulbs, :none?
+    end
+  end
+
+  test "active conter cache" do
+    car = Car.new
+    car.tyres = [Tyre.new, Tyre.new]
+    car.save!
+
+    assert_equal 2, car.custom_tyres_count
+    car.reload
+
+    assert_no_queries do
+      assert_equal 2, car.tyres.size
+      assert_not_predicate car.tyres, :empty?
+      assert_predicate car.tyres, :any?
+      assert_not_predicate car.tyres, :none?
+    end
+
+    assert_queries_count(1) do
+      assert_equal 2, car.tyres.count
+    end
+  end
+
   private
     def assert_touching(record, *attributes)
       record.update_columns attributes.index_with(5.minutes.ago)

activerecord/test/fixtures/cars.yml

@@ -2,10 +2,14 @@ honda:
   id: 1
   name: honda
   engines_count: 0
+  bulbs_count: 0
+  custom_tyres_count: 0
   person_id: 1
 
 zyke:
   id: 2
   name: zyke
   engines_count: 0
+  bulbs_count: 0
+  custom_tyres_count: 0
   person_id: 2

activerecord/test/models/bulb.rb

@@ -2,7 +2,7 @@
 
 class Bulb < ActiveRecord::Base
   default_scope { where(name: "defaulty") }
-  belongs_to :car, touch: true
+  belongs_to :car, touch: true, counter_cache: { active: false }
   scope :awesome, -> { where(frickinawesome: true) }
 
   attr_reader :scope_after_initialize, :attributes_after_initialize, :count_after_create

activerecord/test/models/car.rb

@@ -14,7 +14,7 @@ class Car < ActiveRecord::Base
 
   has_one :bulb
 
-  has_many :tyres
+  has_many :tyres, counter_cache: :custom_tyres_count
   has_many :engines, dependent: :destroy, inverse_of: :my_car
   has_many :wheels, as: :wheelable, dependent: :destroy
 

activerecord/test/models/tyre.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Tyre < ActiveRecord::Base
-  belongs_to :car
+  belongs_to :car, counter_cache: { active: true, column: :custom_tyres_count }
 
   def self.custom_find(id)
     find(id)