Strong Migrations errors

[therealrodk]

When running the included migrations, the user encounters several breaking errors presented by #strong_migrations.

They are: Migration #1: None

Migration #2:

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingUniqueIndices < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index ActsAsTaggableOn.tags_table, :name, unique: true, algorithm: :concurrently
  end
end

Migration #3:

=== Dangerous operation detected #strong_migrations ===

Adding a column with a non-null default causes the entire table to be rewritten.
Instead, add the column without a default value, then change the default.

class AddTaggingsCounterCacheToTags < ActiveRecord::Migration[6.0]
  def up
    add_column :tags, :taggings_count, :integer
    change_column_default :tags, :taggings_count, 0
  end

  def down
    remove_column :tags, :taggings_count
  end
end

Then backfill the existing rows in the Rails console or a separate migration with disable_ddl_transaction!.

class BackfillAddTaggingsCounterCacheToTags < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    Tag.unscoped.in_batches do |relation|
      relation.update_all taggings_count: 0
      sleep(0.1)
    end
  end
end

Migration #4:

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingTaggableIndex < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index :taggings, [:taggable_id, :taggable_type, :context], name: "taggings_taggable_context_idx", algorithm: :concurrently
  end
end

and then subsequently:

Index name 'taggings_taggable_context_idx' on table 'taggings' already exists

because this apparently duplicates an index created earlier.

Migration #5: None.

Migration #6:

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingIndexesOnTaggings < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index :taggings, :taggable_id, algorithm: :concurrently
  end
end

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingIndexesOnTaggings < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index :taggings, :taggable_type, algorithm: :concurrently
  end
end

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingIndexesOnTaggings < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index :taggings, :tagger_id, algorithm: :concurrently
  end
end

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingIndexesOnTaggings < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index :taggings, :context, algorithm: :concurrently
  end
end

=== Dangerous operation detected #strong_migrations ===

Adding an index non-concurrently locks the table. Instead, use:

class AddMissingIndexesOnTaggings < ActiveRecord::Migration[6.0]
  disable_ddl_transaction!

  def change
    add_index :taggings, [:tagger_id, :tagger_type], algorithm: :concurrently
  end
end

=== Best practice #strong_migrations ===

Adding a non-unique index with more than three columns rarely improves performance.
Instead, start an index with columns that narrow down the results the most.

The last error refers to this line:

unless index_exists? ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :tagger_id, :context], name: 'taggings_idy'
      add_index ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :tagger_id, :context], name: 'taggings_idy'
    end

I am using acts-as-taggable-on version 6.5 (from RubyGems), ruby 2.7, and rails 6.0.2.1.

[ryanchin]

To create a workaround, I ended up wrapping each line strong parameters had a problem with, in safety_assured { }.

[Willardgmoore]

@ryanchin How many lines did you wrap with that? I was unaware of that method until I stumbled on your reply. This was the only one that I used that wrapper on.

/db/migrate/20200412171364_add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb:22:in `change'

I found that adding disable_ddl_transaction! and , algorithm: :concurrently as needed solved the other issues as I experienced them.

[ryanchin]

@Willardgmoore I kind of did it one by one, as I got the errors during migration. Most likely, for everyone one you added , algorithm: :concurrently to, I did the same with safety_assured.

I tried the way you did it. too, but I didn't wrap it correctly and kept getting errors (so I looked for a different method).

[damienlethiec]

Same error here

[MohamedAlaa]

For anyone who is facing this issue, I fixed my migrations as follow:

add_missing_unique_indices.acts_as_taggable_on_engine.rb

# This migration comes from acts_as_taggable_on_engine (originally 2)
if ActiveRecord.gem_version >= Gem::Version.new('5.0')
  class AddMissingUniqueIndices < ActiveRecord::Migration[4.2]; end
else
  class AddMissingUniqueIndices < ActiveRecord::Migration; end
end
AddMissingUniqueIndices.class_eval do
  def self.up
    safety_assured {
      add_index ActsAsTaggableOn.tags_table, :name, unique: true unless index_exists?(ActsAsTaggableOn.taggings_table, :name)

      remove_index ActsAsTaggableOn.taggings_table, :tag_id if index_exists?(ActsAsTaggableOn.taggings_table, :tag_id)
      remove_index ActsAsTaggableOn.taggings_table, name: 'taggings_taggable_context_idx' if index_exists?(ActsAsTaggableOn.taggings_table, name: 'taggings_taggable_context_idx')
      add_index ActsAsTaggableOn.taggings_table,
                [:tag_id, :taggable_id, :taggable_type, :context, :tagger_id, :tagger_type],
                unique: true, name: 'taggings_idx'
    }
  end

  def self.down
    safety_assured {
      remove_index ActsAsTaggableOn.tags_table, :name

      remove_index ActsAsTaggableOn.taggings_table, name: 'taggings_idx'

      add_index ActsAsTaggableOn.taggings_table, :tag_id unless index_exists?(ActsAsTaggableOn.taggings_table, :tag_id)
      add_index ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :context], name: 'taggings_taggable_context_idx'
    }
  end
end

add_taggings_counter_cache_to_tags.acts_as_taggable_on_engine.rb

# This migration comes from acts_as_taggable_on_engine (originally 3)
if ActiveRecord.gem_version >= Gem::Version.new('5.0')
  class AddTaggingsCounterCacheToTags < ActiveRecord::Migration[4.2]; end
else
  class AddTaggingsCounterCacheToTags < ActiveRecord::Migration; end
end
AddTaggingsCounterCacheToTags.class_eval do
  def self.up
    add_column ActsAsTaggableOn.tags_table, :taggings_count, :integer
    change_column_default ActsAsTaggableOn.tags_table, :taggings_count, 0

    ActsAsTaggableOn::Tag.reset_column_information
    ActsAsTaggableOn::Tag.find_each do |tag|
      ActsAsTaggableOn::Tag.reset_counters(tag.id, ActsAsTaggableOn.taggings_table)
    end
  end

  def self.down
    remove_column ActsAsTaggableOn.tags_table, :taggings_count
  end
end

add_missing_taggable_index.acts_as_taggable_on_engine.rb This one is a useless migration because the index is trying to create already has been created earlier.

# This migration comes from acts_as_taggable_on_engine (originally 4)
if ActiveRecord.gem_version >= Gem::Version.new('5.0')
  class AddMissingTaggableIndex < ActiveRecord::Migration[4.2]; end
else
  class AddMissingTaggableIndex < ActiveRecord::Migration; end
end
AddMissingTaggableIndex.class_eval do
  disable_ddl_transaction!

  def change
    safety_assured {
      # this index has already been created before.
      # add_index ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :context], name: 'taggings_taggable_context_idx', algorithm: :concurrently unless index_exists?(ActsAsTaggableOn.taggings_table, name: 'taggings_taggable_context_idx')
    }
  end
end

add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb

# This migration comes from acts_as_taggable_on_engine (originally 6)
if ActiveRecord.gem_version >= Gem::Version.new('5.0')
  class AddMissingIndexesOnTaggings < ActiveRecord::Migration[4.2]; end
else
  class AddMissingIndexesOnTaggings < ActiveRecord::Migration; end
end
AddMissingIndexesOnTaggings.class_eval do
  disable_ddl_transaction!

  def change
    safety_assured {
      add_index ActsAsTaggableOn.taggings_table, :tag_id unless index_exists? ActsAsTaggableOn.taggings_table, :tag_id

      unless index_exists? ActsAsTaggableOn.taggings_table, :taggable_id
        add_index ActsAsTaggableOn.taggings_table, :taggable_id, algorithm: :concurrently
      end

      unless index_exists? ActsAsTaggableOn.taggings_table, :taggable_type
        add_index ActsAsTaggableOn.taggings_table, :taggable_type, algorithm: :concurrently
      end

      unless index_exists? ActsAsTaggableOn.taggings_table, :tagger_id
        add_index ActsAsTaggableOn.taggings_table, :tagger_id, algorithm: :concurrently
      end

      unless index_exists? ActsAsTaggableOn.taggings_table, :context
        add_index ActsAsTaggableOn.taggings_table, :context, algorithm: :concurrently
      end

      unless index_exists? ActsAsTaggableOn.taggings_table, [:tagger_id, :tagger_type]
        add_index ActsAsTaggableOn.taggings_table, [:tagger_id, :tagger_type], algorithm: :concurrently
      end

      unless index_exists? ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :tagger_id, :context], name: 'taggings_idy'
        add_index ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :tagger_id, :context], name: 'taggings_idy', algorithm: :concurrently
      end
    }
  end
end

I will try to make a new pull request to submit this fix.