lucky_migrator icon indicating copy to clipboard operation
lucky_migrator copied to clipboard

Consider adding method for easily creating counter caches

Open paulcsmith opened this issue 7 years ago • 3 comments

http://shuber.io/porting-activerecord-counter-cache-behavior-to-postgres/

https://github.com/magnusvk/counter_culture/blob/master/README.md

https://github.com/jeremyevans/sequel_postgresql_triggers

paulcsmith avatar Dec 24 '17 01:12 paulcsmith

I actually implemented this in one of my projects, I'll post the code here for reference. It was adapted from the shuber.io link to handle polymorphic associations.

I declared the counter cache functions in a migration.

class CreateCounterCache < ActiveRecord::Migration[5.1]
  def up
    execute %{
      -- Increment's the integer value of a column in the given table
      CREATE OR REPLACE FUNCTION increment_counter(table_name text, column_name text, id integer, step integer)
        RETURNS VOID AS $$
          DECLARE
            table_name text := quote_ident(table_name);
            column_name text := quote_ident(column_name);
            conditions text := ' WHERE id = $1';
            updates text := column_name || '=' || column_name || '+' || step;
          BEGIN
            EXECUTE 'UPDATE ' || table_name || ' SET ' || updates || conditions
            USING id;
          END;
        $$ LANGUAGE plpgsql;

      CREATE OR REPLACE FUNCTION counter_cache()
        RETURNS trigger AS $$
          DECLARE
            counter_name text := quote_ident(TG_ARGV[0]);
            fk_table text := quote_ident(TG_ARGV[1]);
            fk_name text := quote_ident(TG_ARGV[2]);
            fk_changed boolean := false;
            fk_value integer;
            table_name text;
            record record;
          BEGIN
            IF TG_OP = 'UPDATE' THEN
              record := NEW;
              EXECUTE 'SELECT ($1).' || fk_name || ' != ' || '($2).' || fk_name
              INTO fk_changed
              USING OLD, NEW;
            END IF;

            IF TG_OP = 'DELETE' OR fk_changed THEN
              record := OLD;
            END IF;

            IF TG_OP = 'INSERT' OR fk_changed THEN
              record := NEW;
            END IF;

            IF TG_OP = 'DELETE' OR TG_OP = 'INSERT' OR fk_changed THEN
              EXECUTE 'SELECT ($1).' || fk_name INTO fk_value USING record;

              EXECUTE 'SELECT lower(($1).' || fk_table || ') || ' || '''s''' INTO table_name USING record;
            END IF;

            IF TG_OP = 'DELETE' OR fk_changed THEN
              PERFORM increment_counter(table_name, counter_name, fk_value, -1);
            END IF;

            IF TG_OP = 'INSERT' OR fk_changed THEN
              PERFORM increment_counter(table_name, counter_name, fk_value, 1);
            END IF;

            RETURN record;
          END;
        $$ LANGUAGE plpgsql;
    }
  end

  def down
    execute %{
      DROP FUNCTION IF EXISTS counter_cache();
      DROP FUNCTION IF EXISTS increment_counter(table_name text, column_name text, id integer, step integer);
    }
  end
end

Then used them on a table like this:

CREATE TRIGGER update_reviews_count
      AFTER INSERT OR UPDATE OR DELETE ON reviews
      FOR EACH ROW EXECUTE PROCEDURE counter_cache('reviews_count', 'reviewable_type', 'reviewable_id');

All of this was done through migrations at the database level, which is fantastic but how would it be done in Lucky? Would we have a macro that checks the tables for the right columns and creates the trigger? Maybe like:

class Review < LuckyRecord::Model
  counter_cache Restaurant, :reviews_count, :review_id
end

And this could check the Restaurant model for a review_id field, possibly after running ensure_correct_field_mappings!. This would also need to CREATE OR REPLACE the functions, and make sure the trigger is added to reviews.

Anyway regardless how we do it, I'm interested in implementing it.

mikeeus avatar Dec 24 '17 11:12 mikeeus

I would love some help with this :D And that's exactly the type of thing I'd like to see in Migrator.

I think this could be done purely in Migrator without LuckyRecord. I think I'd like to add somthing like https://github.com/jeremyevans/sequel_postgresql_triggers#counter-cache---pgt_counter_cache. Example:

def migrate
  create_counter_cache count: :comments, with: :post_id, stored_in_table: :posts,  stored_in_column: :comments_count
end

And maybe some options for other custom SQL if you want to limit which rows are counted (like ignored arhcived = true or something.

Does something like that make sense?

paulcsmith avatar Jan 09 '18 16:01 paulcsmith

Are you looking for something like this?

require "./spec_helper.cr"

class AddCounterCacheOnUserComments::V001 < LuckyMigrator::Migration::V1
  def migrate
    create_counter_cache count: :comments, with: :user_id, stored_in_table: :users, stored_in_column: :comments_count, name: :cc_users_comments_count
  end

  def rollback
    drop_counter_cache :cc_users_comments_count
  end
end

describe LuckyMigrator::Trigger do
  describe "counter_cache" do
    it "creates counter_cache" do
      AddCounterCacheOnUserComments::V001.new.up

      users_triggers = LuckyRecord::Repo.table_triggers("users")
      users_triggers.should eq ["users_comments_count_counter_cache"] # some name

      # create user
      users.comments_count.should eq 0 # column exists
      # create user comment
      users.comments_count.should eq 1
      # delete user comment;
      users.comments_count.should eq 0
    end

    it "drops the trigger on rollback" do
      AddCounterCacheOnUserComments::V001.new.down

      users_triggers = LuckyRecord::Repo.table_triggers("users")
      users_triggers.should be_empty

      # create user
      # create user comment;
      users.comments_count.should eq 0 # Or should we remove the column?
    end
  end
end

# LuckyRecord::Repo.table_triggers
def table_triggers(table) : Array(String)
  LuckyRecord::Repo.run do |db|
    db.query_one? <<-SQL
    SELECT tgname
    FROM pg_trigger
    WHERE NOT tgisinternal
    AND tgrelid = '#{table}'::regclass;
    SQL, as: Array(String)
  end
end

I added a name option to make it easy to reference for dropping it on rollback.

Also what do you think about creating the counter column with ALTER TABLE users ADD COLUMN IF NOT EXISTS comments_count INTEGER DEFAULT 0;? Then adding an option to drop_counter_cache like drop_column or something to that effect.

mikeeus avatar Jan 10 '18 06:01 mikeeus