I am planning to drop support for rails < 6.1 and ruby < 2.7 in the future. Does anyone use this gem with these old versions?

The Adding multiple foreign keys section writes:

Adding multiple foreign keys in a single migration blocks reads and writes on all involved tables until migration is completed.

@fatkodima has also written that adding a single foreign key is safe:

Adding a single foreign key when creating a table is safe, but adding multiple foreign keys is not, as you actually experienced. See https://github.com/fatkodima/online_migrations#adding-multiple-foreign-keys for the details.

1. Why does adding 2 foreign keys in the same create_table block reads and writes on all involved tables? Specifically, I'm surprised by the blocks reads part. I thought SHARE ROW EXCLUSIVE locks would be acquired for all involved tables, but that lock should not block reads.
2. why is adding more than 1 foreign key in a single migration unsafe, while adding exactly 1 foreign key is safe? Is it due to needing to acquire more locks on different tables, how the postgres lock queue works, and the migration block not finishing and releasing all acquired keys until all foreign keys are added? But waiting to finish and releasing keys are needed in the 1 foreign key case too, no?

The following error when creating an index currently at the PostgreSQL versions 14.0 to 14.3 is not being raised on the local environment.

StandardError: An error has occurred, all later migrations canceled:

⚠️  [online_migrations] Dangerous operation detected ⚠️

Adding an index concurrently can cause silent data corruption in PostgreSQL 14.0 to 14.3.
Upgrade PostgreSQL before adding new indexes, or wrap this step in a safety_assured { ... }
block to accept the risk.

Do you think that this approach is the better one? Not raising this error in the local environment might make us believe that our migration is safe to run on production (I'm reverting a PR that only raised this error on staging env). What do you think of changing this behaviour or maybe raising a warning(Utils.warn)?

I'm seeking clarification about the lock retry behaviour when running the migration outside of a transaction.

In the docs you explicitly state the retry behaviour when the migration runs inside a migration

When statement within transaction fails - the whole transaction is retried.

However when running the migration without a transaction via disable_ddl_transaction! it's not clear in the docs but my assumption is that the LockRetrier does the same. E.g. in the test code it shows the migration is retried by the LockRetrier mechanism (if set).

Can you please clarify the behaviour is as I assume. I'm happy to get a PR up to update the configuring.md doc as well.

Description

Hi @fatkodima,

Not sure if you would like to elaborate more on those descriptions here:
https://github.com/fatkodima/online_migrations#migration-timeouts
https://github.com/ankane/strong_migrations/blob/master/README.md#good-7

For instance like concurrent indexing, the requirement for longer statement timeout might be needed since the fact that statement could run for quite a while on larger table, apart from the application level. I think this has already implemented as a good practice in few of the larger organizations and gems.

REF:
https://gitlab.com/gitlab-org/gitlab-foss/-/blob/f21e655b61725b972ae80d584a66d6deb53a337d/lib/gitlab/database/migration_helpers.rb#L68
https://github.com/doctolib/safe-pg-migrations/blob/1ea078352413df201a4d410f5947ece1c67923fd/test/idempotent_statements_test.rb#L102-L124
https://github.com/ankane/strong_migrations#migration-timeouts

Hi :)

Found a small issue when using the add_not_null_constraint helper: it does not quote the column name in the check constraint expression. So, if your column is also a reserved keyword, adding the check constraint will fail.

For example, the following would not work:

add_not_null_constraint 'foos', 'group'

Here, group is also a reserved keyword. The generated check constraint expression should be 'group' IS NOT NULL rather than group IS NOT NULL.

I believe the fix is simple, all you have to change is this line: https://github.com/fatkodima/online_migrations/blob/master/lib/online_migrations/schema_statements.rb#L510:

expression = "'#{column_name}' IS NOT NULL"

I tried creating a PR myself, but could not get the test suite working locally.

Thanks in advance!

ref: https://github.com/fatkodima/online_migrations#adding-a-foreign-key
ref: https://github.com/ankane/strong_migrations#good-10

We were wondering if the suggestion to do this in a single migration is accurate. Consider a hypothetical situation where validate_foreign_key raises - for example, due to a timeout.

The migration will complete unsuccessfully, so the schema_migrations table won't get updated. Because of disable_ddl_transaction! it won't get rolled back. When you run db:migrate again, it'll try to re-run the same migration, but add_foreign_key will fail because the foreign key already exists.

Does online_migrations do something magical to handle this issue automatically? Or are we understanding the issue wrong?

I've been able to identify the root cause of the issue and I have a PR coming.

I have another column on my table that includes the name of the column I'm actually trying to copy the index for, and that column is JSONB. For example, I am trying to copy the index for foo but I also have a JSONB column named foo_bar. The foo_bar column has a JSONB index, which Rails interprets as plain text rather than an array. See:

          #<ActiveRecord::ConnectionAdapters::IndexDefinition:0x00000001205e9748
          # @columns="(((foo_bar -> 'session'::text) ->> 'note'::text))",
          # @comment=nil,
          # @lengths={},
          # @name="index_notes_on_foo_bar_note",
          # @opclasses={},
          # @orders={},
          # @table="notes",
          # @type=nil,
          # @unique=false,
          # @using=:btree,
          # @where=nil>]

online_migrations assumes #columns is always an array, but in this case it is a String. We should only handle arrays.

Hi,

First, thanks a lot for your hard work on this gem! It is very useful to us.

However, we ran into a snag when trying to add a deferred foreign key using the add_foreign_key helper. It appears that this gem alters this helper (for instance, to drop a pre-existing foreign key with the same name), but in doing so it breaks the deferrable option. Only way to add deferred foreign keys is by doing it by hand with a SQL statement. Less than ideal ofcourse ;-)

We use Rails 7.0.4.2.

Description

Hi @fatkodima, thanks for this nice gem, currently we've used rails db:structure:load for the CI integration/tests, which kind of avoid the checking migrations created from other team members, is it possible to check the new migrations manually from a rake tasks? Thanks!

Currently I am investigating one issues I had with my app. Let consider the migation like ⬇️

class MyMigration < ActiveRecord::Migration[6.1]
  disable_ddl_transaction!

  def change
    add_index :some_big_table, [:column_a, :column_b], algorithm: :concurrently

    remove_index :some_big_table, :column_a, algorithm: :concurrently
  end

end

At first glance everything is fine as this is concurrently, but under the hood Postgres locks the table with share update exclusive to prevent simultaneous schema changes. Wouldn't it be good if online_migrations warn about such behaviour and advice? Is it aligned with vision for this tool?

After running a migration that uses the initialize_column_rename helper method, our unit tests that get the most recently created record with code like SomeModel.last started failing because they were returning fixture records that were inserted before the tests.

Some speculation: perhaps Rails (or maybe minitest) resets Postgres sequence numbers when setting up tests. I'm guessing that normally Rails uses the max ID value for a table to set its related sequence. When the table has been temporarily renamed and a view instead represents the table this mechanism appears to be interfered with. Instead of the table's sequence's next value being set to 593363171 (or whatever, based on the test fixtures already inserted into the table), it's set to 1.

Note that we do have this configuration code:

OnlineMigrations.configure do |config|
  ...
  config.column_renames = {
    'some_models' => {
      'foo' => 'bar',
    },
  }

We've fixed this problem by replacing SomeModel.last with SomeModel.order(:created_at).last in our tests, but a solution that doesn't require changing test code would be nice. If that makes sense and I can provide any extra information, please let me know. I'd also be open to looking into a fix, though I'm currently unclear on the mechanics behind both the column_renames support and Rails' modifications to the sequence value.

Hello

When adding online_migrations on an old project. I have to change previous and old migrations when the database was not populated so without any risk of issue. Is it possible to make check on migration only on migration after a specific date?

I can work on this PR if needed.

When changing the type of a not null column with a default, online_migrations is able to avoid having to update pg_catalog ... Which is great because privileges on that table are restricted in many environments.

However, if the column in question does not have a default, we drop into the codepath in finalize_columns_type_change that attempts to update pg_catalog.

A similar concern was raised in #21 but i believe the changes there addressed PK columns and those with defaults.

Steps to recreate

1. On postgresql >= version 12, follow the steps to change the type of a column.
2. Use a column with null: false but without a default.
3. Run migrations (with config.verbose_sql_logs = true).
4. In stdout or logs, look for UPDATE pg_catalog.pg_attribute.

Snippets from my testing

❯ postgres --version
postgres (PostgreSQL) 13.11

  # first migration: changing `number` from `string` to `integer`
  # the original column is `null: false`
  def change
    initialize_column_type_change :some_numbers, :number, :integer
  end

Running the 'finalize' migration shows:

D, [2023-09-23T23:43:47.667897 #24329] DEBUG -- :   TRANSACTION (0.0ms)  BEGIN /*application:Wrapbook*/
D, [2023-09-23T23:43:47.668343 #24329] DEBUG -- :    (0.2ms)  UPDATE pg_catalog.pg_attribute
SET attnotnull = true
WHERE attrelid = 'some_numbers'::regclass
  AND attname = 'number_for_type_change'
 /*application:Wrapbook*/
D, [2023-09-23T23:43:47.669486 #24329] DEBUG -- :    (0.2ms)  ALTER TABLE "some_numbers" DROP CONSTRAINT "chk_rails_0171ff2f52" /*application:Wrapbook*/
D, [2023-09-23T23:43:47.669940 #24329] DEBUG -- :   TRANSACTION (0.2ms)  COMMIT /*application:Wrapbook*/

This works on my local but fails in managed environments.

Workaround

1. Force a default value in the initialize migration, eg:
   initialize_column_type_change :some_numbers, :number, :integer, null: false, default: 0
2. Add remaining migrations as prescribed.
3. Add an extra migration to drop the default after the 'cleanup' step, eg:
   change_column_default :some_numbers, :number, nil

While this a relatively easy workaround, i think the issue might be worth fixing because it can be a bit baffling when first encountered.

Possible fix (?)

The comment here speaks to the possibility of promoting the check constraint on PG >= 12, instead of attempting to update pg_catalog. Perhaps we need to implement that to cover this edge case?

I'd be happy to attempt that in a PR if you think that's the right direction.

Thanks for all your work on this very useful gem!

Ruby version: 3.2.2
Rails version: 7.0.8
Gem version: 0.15.0

When migrating a primary key data type change from integer to bigint using the helper backfill_column_for_type_change_in_background:

initialize_column_type_change forces a default 0 value for reasons mentioned:
https://github.com/fatkodima/online_migrations/blame/0a1f67fc44b130f1ed58f77d7b8b25bf63634f72/lib/online_migrations/change_column_type_helpers.rb#L129

However this results in a bad query down the line in the relation used:

This results in a query that attempts to copy from a table expecting null, but we have backfilled it with 0:

development> stmt.to_sql
=> "UPDATE \"projects\" SET \"id_for_type_change\" = \"projects\".\"id\" WHERE \"projects\".\"id_for_type_change\" IS NULL AND \"projects\".\"id\" IS NOT NULL"

Consequently, the background migration job completes successfully but doesn't actually copy any values to the new column.

Reproduction:

1. Create a migration to initialize a type change on any primary key for any table:

initialize_column_type_change :projects, :id, :bigint

2. Populate the background migration record for the background job cron to pick up:

backfill_column_for_type_change_in_background :projects, :id

3. Migration and job completes with status successful but the new column created id_for_type_change is still all 0 value.

Hi,

We have some indexes on a table that were created using expressions, for instance:

class AddLocaleDomainIndex < ActiveRecord::Migration[5.2]
  def change
    add_index :organizations, 'avals(locale_domain)', using: :gin, name: 'index_organizations_on_locale_domain'
  end
end

Now, when we try to remove a column from this table without using safety_assured, the CommandChecker crashes as follows:

undefined method `&' for "avals(locale_domain)":String
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/online_migrations-0.7.2/lib/online_migrations/command_checker.rb:389:in `block in check_columns_removal'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/online_migrations-0.7.2/lib/online_migrations/command_checker.rb:388:in `select'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/online_migrations-0.7.2/lib/online_migrations/command_checker.rb:388:in `check_columns_removal'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/online_migrations-0.7.2/lib/online_migrations/command_checker.rb:116:in `do_check'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/online_migrations-0.7.2/lib/online_migrations/command_checker.rb:34:in `check'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/online_migrations-0.7.2/lib/online_migrations/migration.rb:21:in `method_missing'
/Users/bookingexperts/Code/monorepo/apps/parkcms/db/migrate/20230530085532_remove_module_multi_booking_column_from_organizations.rb:3:in `up'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/activerecord-7.0.4.3/lib/active_record/migration.rb:873:in `public_send'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/activerecord-7.0.4.3/lib/active_record/migration.rb:873:in `exec_migration'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/activerecord-7.0.4.3/lib/active_record/migration.rb:854:in `block (2 levels) in migrate'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/2.7.0/benchmark.rb:293:in `measure'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/activerecord-7.0.4.3/lib/active_record/migration.rb:853:in `block in migrate'
/Users/bookingexperts/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/activerecord-7.0.4.3/lib/active_record/connection_adapters/abstract/connection_pool.rb:215:in `with_connection'

Seems like the CommandChecker uses connection.indexes, which can also contain expressions as strings, for PostgreSQL at least:

https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L118

Thanks in advance for looking into this!

First of all thank you for your hard work on this gem!

I noticed that the ability to remove index by name doesn't work as it does using the Rails remove_index method.

example:

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

  def up
    add_index :campaigns, :weight, algorithm: :concurrently
  end

  def down
    remove_index :campaigns, name: :index_campaigns_on_weight, algorithm: :concurrently
  end
end

The above, after running migrate (successfully adds index), then rollback results in:

-- remove_index(:campaigns, {:name=>:index_campaigns_on_weight, :algorithm=>:concurrently})
-- [online_migrations] Index was not removed because it does not exist (this may be due to an aborted migration or similar): table_name: campaigns, column_name: []
   -> 0.0040s

As a side note, its quite inconvenient that this sets the migration as completed, though the index was not removed.

Similarly to #26, this issue happens after running the finalize_column_rename migration, but before deploying the final code changes. Sentry logged the following error:

ActiveRecord::StatementInvalid /organizations
PG::UndefinedTable: ERROR: relation "organizations_column_rename" does not exist LINE 9: WHERE a.attrelid = '"organizations_column_rename"'::regclas... ^

PG::UndefinedTable
ERROR:  relation "organizations_column_rename" does not exist
LINE 9:  WHERE a.attrelid = '"organizations_column_rename"'::regclas...
                            ^
Crashed in non-app: activerecord (7.0.4.2) lib/active_record/connection_adapters/postgresql/database_statements.rb in exec
Called from: online_migrations (0.5.4) lib/online_migrations/schema_cache.rb in columns at line 20
Called from: activerecord (7.0.4.2) lib/active_record/connection_adapters/schema_cache.rb in block in columns_hash
lib/middleware/rewriter/middleware.rb in thread_safe_call at line 41
lib/middleware/rewriter/middleware.rb in call at line 25
Called from: sentry-rails (5.8.0) lib/sentry/rails/rescued_exception_interceptor.rb in call
lib/middleware/gcp_trace_middleware.rb in call at line 12
Called from: rack (2.2.6.2) lib/rack/method_override.rb in call

The mentioned line of the online_migrations gem is this:

This is only a hunch, but maybe this issue happens because the previous line

elsif renamed_columns.key?(table_name)

incorrectly retrieves true. The renamed_columns method has a caching that might cause the problem here. I could not verify yet, but maybe the remove of the cache would solve the problem, so that method could look like that:

    def renamed_columns
      column_renames = OnlineMigrations.config.column_renames
      return {} if column_renames.blank?

      views = connection.views
      column_renames.select do |table_name, _|
        views.include?(table_name)
      end
    end

What do you think?

A couple questions about background migrations https://github.com/fatkodima/online_migrations/blob/master/docs/background_migrations.md:

1. why does it have its own entire system, why not just make an ActiveJob and let the normal background job processor do the work?
2. are the phases aware of if the data migration jobs have finished? For example, for https://github.com/fatkodima/online_migrations#changing-the-type-of-a-column, does finalize_column_type_change "know" if the background version of backfill_column_for_type_change has finished yet, and refuse to run if it hasn't?

In our situation, we needed to rename multiple columns in the same table, and it seems online_migrations does not support this currently:

OnlineMigrations.config.column_renames["users"] = {
  "fname" => "first_name",
  "lname" => "last_name"
}

The example above would work only for fname rename.

Is it possible to get official support for this?

To solve the situation quickly for our case, we monkey-patched two modules of the gem:

schema_cache.rb:

module OnlineMigrations
  module SchemaCache
    def columns(table_name)
      if renamed_tables.key?(table_name)
        super(renamed_tables[table_name])
      elsif renamed_columns.key?(table_name)
        columns = super(column_rename_table(table_name))
        renamed_columns[table_name].each_key do |old_column_name|
          new_column_name = renamed_columns[table_name][old_column_name]
          duplicate_column(old_column_name, new_column_name, columns)
        end
        columns
      else
        super.reject { |column| column.name.end_with?("_for_type_change") }
      end
    end

    def duplicate_column(old_column_name, new_column_name, columns)
      old_column = columns.find { |column| column.name == old_column_name }
      new_column = old_column.dup
      # ActiveRecord defines only reader for :name
      new_column.instance_variable_set(:@name, new_column_name)
      # Correspond to the ActiveRecord freezing of each column
      columns << new_column.freeze
    end
  end
end

schema_statements.rb:

module OnlineMigrations
  module SchemaStatements
    # Renames a column without requiring downtime
    #
    # The technique is built on top of database views, using the following steps:
    #   1. Rename the table to some temporary name
    #   2. Create a VIEW using the old table name with addition of the new columns as an aliases of the old ones
    #   3. Add a workaround for ActiveRecord's schema cache
    #
    # @param table_name [String, Symbol] table name
    # @param old_new_column_hash [Hash] the hash of old and new columns
    #
    # @return [void]
    #
    # @example
    #   initialize_column_renames(:users, {fname: :first_name, lname: :last_name})
    #
    # @note
    #   Prior to using this method, you need to register the database table so that
    #   it instructs ActiveRecord to fetch the database table information (for SchemaCache)
    #   using the original table name (if it's present). Otherwise, fall back to the old table name:
    #
    #   ```OnlineMigrations.config.column_renames[table_name] = { old_column_name => new_column_name }```
    #
    #   Deploy this change before proceeding with this helper.
    #   This is necessary to avoid errors during a zero-downtime deployment.
    #
    # @note None of the DDL operations involving original table name can be performed
    #   until `finalize_column_rename` is run
    #
    def initialize_column_renames(table_name, old_new_column_hash)
      transaction do
        rename_table_create_view(table_name, old_new_column_hash)
      end
    end

    # Reverts operations performed by initialize_column_renames
    #
    # @param table_name [String, Symbol] table name
    # @param _old_new_column_hash [Hash] the hash of old and new columns
    #     Passing this argument will make this change reversible in migration
    #
    # @return [void]
    #
    # @example
    #   revert_initialize_column_renames(:users, {fname: :first_name, lname: :last_name})
    #
    def revert_initialize_column_renames(table_name, _old_new_column_hash = nil)
      transaction do
        execute("DROP VIEW #{table_name}")
        rename_table("#{table_name}_column_rename", table_name)
      end
    end

    # Finishes the process of column renames
    #
    # @param (see #initialize_column_renames)
    # @return [void]
    #
    # @example
    #   finalize_column_renames(:users, {fname: :first_name, lname: :last_name})
    #
    def finalize_column_renames(table_name, old_new_column_hash)
      transaction do
        execute("DROP VIEW #{table_name}")
        rename_table("#{table_name}_column_rename", table_name)
        old_new_column_hash.each_key do |old|
          rename_column(table_name, old, old_new_column_hash[old])
        end
      end
    end

    # Reverts operations performed by finalize_column_renames
    #
    # @param (see #initialize_column_renames)
    # @return [void]
    #
    # @example
    #   revert_finalize_column_renames(:users, {fname: :first_name, lname: :last_name})
    #
    def revert_finalize_column_renames(table_name, old_new_column_hash)
      transaction do
        old_new_column_hash.each_key do |old|
          rename_column(table_name, old_new_column_hash[old], old)
        end
        rename_table_create_view(table_name, old_new_column_hash)
      end
    end

    private

    def rename_table_create_view(table_name, old_new_column_hash)
      tmp_table = "#{table_name}_column_rename"
      rename_table(table_name, tmp_table)
      column_mapping = old_new_column_hash.keys.map { |old| "#{old} AS #{old_new_column_hash[old]}" }.join(", ")
      execute("CREATE VIEW #{table_name} AS SELECT *, #{column_mapping} FROM #{tmp_table}")
    end
  end
end

I wonder if a change like that can go into the codebase of online_migrations ?

I have a situation like this:

application.rb:

  class Application < Rails::Application
    # Initialize configuration defaults for originally generated Rails version.
    config.load_defaults 7.0

    # Allow partial inserts
    config.active_record.partial_inserts = true

    # Enumerate columns in select statements
    config.active_record.enumerate_columns_in_select_statements = true
  end

initializers/online_migrations.rb:

# initializer
OnlineMigrations.config.column_renames = {
  "users" => {
    "name" => "first_name"
  }
}

The first two parts of the rename operation have been already executed, and I have the migration file ready for the final part:

20230214162556_finalize_rename.rb:

class FinalizeRename < ActiveRecord::Migration[7.0]
  def change
    finalize_column_rename :users, :name, :first_name
  end
end

Now I open a ruby console and execute

> User.first
  User Load (0.3ms)  SELECT "users"."id", "users"."name", "users"."created_at", "users"."updated_at", "users"."first_name" FROM "users" ORDER BY "users"."id" ASC LIMIT $1  [["LIMIT", 1]]
 => #<User:0x00000001120919c0 id: 1, name: "Joe", created_at: Tue, 14 Feb 2023 16:13:13.831911000 UTC +00:00, updated_at: Tue, 14 Feb 2023 16:13:13.831911000 UTC +00:00, first_name: "Joe"> 

Then I run the migration in a separate shell:

$ bin/rails db:migrate
== 20230214162556 FinalizeRename: migrating ===================================
-- finalize_column_rename(:users, :name, :first_name)
   -> 0.0309s
== 20230214162556 FinalizeRename: migrated (0.0335s) ==========================

Then I go back to the rails console and execute again:

> User.first
  User Load (0.8ms)  SELECT "users"."id", "users"."name", "users"."created_at", "users"."updated_at", "users"."first_name" FROM "users" ORDER BY "users"."id" ASC LIMIT $1  [["LIMIT", 1]]
/Users/popperur/.rvm/gems/ruby-3.1.2/gems/activerecord-7.0.4.2/lib/active_record/connection_adapters/postgresql_adapter.rb:783:in `exec_prepared': PG::UndefinedColumn: ERROR:  column users.name does not exist at character 22 (ActiveRecord::StatementInvalid)         
/Users/popperur/.rvm/gems/ruby-3.1.2/gems/activerecord-7.0.4.2/lib/active_record/connection_adapters/postgresql_adapter.rb:783:in `exec_prepared': ERROR:  column users.name does not exist at character 22 (PG::UndefinedColumn)

So as you can see, because of the enumerate_columns_in_select_statements = true setting it tries to execute the SQL with an old column. Without that setting the user object is retrieved properly.
I wonder if this is related to the online_migrations gem, and if yes, there is a solution for that? We use that setting to avoid ActiveRecord::PreparedStatementCacheExpired errors.

I had the following migration which I expected to backfill sent_at with the value from created_at, but ended up just creating the new column with null values. Is there a bug or did I do something wrong?

online_migrations 0.5.4
postgresql 14.6
active_record 6.1.7.1

class AddSentAtToPayouts < ActiveRecord::Migration[6.1]
  disable_ddl_transaction!

  def change
    add_column_with_default(
      :payouts,
      :sent_at,
      :timestamp,
      default: Arel.sql('created_at')
    )
  end
end

This works:

    class AddReferenceWithAlgorithmConcurrentlyAndMismatchingType < ActiveRecord::Migration
      disable_ddl_transaction!

      def up
        # fails check cause of type mismatch
        add_reference :projects, :user, type: :uuid, index: { algorithm: :concurrently }
      end

      def down; end
    end

But this doesn't:

    class AddReferenceConcurrentlyWithMismatchingType < ActiveRecord::Migration
      disable_ddl_transaction!

      def up
        # doesn't fail check despite type mismatch
        add_reference_concurrently :projects, :user, type: :uuid 
      end

      def down; end
    end

initialize_column_rename uses a view to facilitate the column name change. However, views are not dumped to db/schema.rb. Hence, if you commit db/schema.rb and use rake db:drop db:create db:schema:load, the resulting database won't work because it is missing the view that stands in for the original table.

This is a problem. It is especially problematic if an environment like CI uses db:schema:load and not migrations to initialize its database. (db:schema:load seems to be the norm for CI environments.)

You can workaround the problem by not committing the db/schema.rb in the interim, using db:schema:load and then running migrations, but this is confusing and error-prone.

The best solution is the addition of the scenic gem to dump views into db/schema.rb.

I can provide a PR if you concur this is the best approach. I have tested this solution locally and it works like a charm with views and schema dumps created in tandem with the use of online_migrations.

Trying to do the basic rename operation:

# initializer
OnlineMigrations.config.column_renames = {
  "users" => {
    "old" => "new"
  }
}

# migration
initialize_column_rename(:users, :old, :new)

It creates the view, I can see it in the schema alright (using structure.sql).

However, if I try

user = User.new
user.save

I get the following error:

ActiveRecord::StatementInvalid: PG::SyntaxError: ERROR:  multiple assignments to same column "old"
...
Caused by PG::SyntaxError: ERROR:  multiple assignments to same column "old"

The insert statement generated by Rails does look wrong:

INSERT INTO users (old, new) VALUES (NULL, NULL)

Versions:

• Rails: 7.0.4.2 (newly generated, blank app)
• Postgres: 9.6 - 14.4
• online_migrations: 0.5.4

Hi, I think I found a bug in update_columns_in_batches. I was running the following migrations, which I'm pretty sure is correct. Pretty sure :D

    # Move settings from hstore to their columns
    columns_with_values = [
      [:enabled_xs, Arel.sql("coalesce((settings -> 'visible_xs')::BOOLEAN, true)")],
      [:enabled_sm, Arel.sql("coalesce((settings -> 'visible_sm')::BOOLEAN, true)")],
      [:enabled_md, Arel.sql("coalesce((settings -> 'visible_md')::BOOLEAN, true)")],
      [:enabled_lg, Arel.sql("coalesce((settings -> 'visible_lg')::BOOLEAN, true)")],
      [:enabled_xl, Arel.sql("coalesce((settings -> 'visible_xl')::BOOLEAN, (settings -> 'visible_lg')::BOOLEAN, true)")]
    ]

    update_columns_in_batches(:widgets, columns_with_values, batch_size: 10_000)
  end

The results, which are incorrect:

Widget.where(enabled_xs: false).count
  Widget Count (281.2ms)  SELECT COUNT(*) FROM "widgets" WHERE "widgets"."enabled_xs" = $1  [["enabled_xs", false]]
=> 20

If I run my migration like this, which I think is the same? I get the correct results:

    # Move settings from hstore to their columns

    update_column_in_batches(:widgets, :enabled_xs, Arel.sql("coalesce((settings -> 'visible_xs')::BOOLEAN, true)"), batch_size: 10_000)
    update_column_in_batches(:widgets, :enabled_sm, Arel.sql("coalesce((settings -> 'visible_sm')::BOOLEAN, true)"), batch_size: 10_000)
    update_column_in_batches(:widgets, :enabled_md, Arel.sql("coalesce((settings -> 'visible_md')::BOOLEAN, true)"), batch_size: 10_000)
    update_column_in_batches(:widgets, :enabled_lg, Arel.sql("coalesce((settings -> 'visible_lg')::BOOLEAN, true)"), batch_size: 10_000)
    update_column_in_batches(:widgets, :enabled_xl, Arel.sql("coalesce((settings -> 'visible_xl')::BOOLEAN, (settings -> 'visible_lg')::BOOLEAN, true)"), batch_size: 10_000)
  end

Widget.where(enabled_xs: false).count
  Widget Count (297.3ms)  SELECT COUNT(*) FROM "widgets" WHERE "widgets"."enabled_xs" = $1  [["enabled_xs", false]]
=> 3097

Index name 'index_blah' on table 'my_table' is too long; the limit is 63 characters

I think coming from here: https://github.com/fatkodima/online_migrations/blob/master/lib/online_migrations/change_column_type_helpers.rb#L446

Is it possible to reuse the name of the existing one, or if it's too long generate a shorter name? Or accept options?

I am a huge fan of this gem, thanks for taking the time to create it.

I have come across another issue while using this to migrate a primary key from integer to bigint that I'm not sure is a problem or not.

Finalize finalize_column_type_change :projects, :id generates an index as follows.

--
-- Name: index_projects_on_((assigned_to ->> 'id_for_type_change'::text)); Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX "index_projects_on_((assigned_to ->> 'id_for_type_change'::text))" ON public.projects USING btree (((assigned_to ->> 'id_for_type_change'::text)));

Subsequently, when the cleanup_column_type_change :projects, :id helper is called, does not remove the above index. Why is this? the id_for_type_change column was removed, so shouldn't this index also be removed?

I am currently following the README to update a column from integer to bigint. Having an issue undefined method cleanup_change_column_type_concurrently when running the migration. Thank you.

Trying to convert a varchar column to integer results in the following error: PG::UndefinedFunction: ERROR: operator does not exist: integer <> character varying

The backfill_column_for_type_change method takes a type_cast_function parameter, but the only example I can find using that passing in jsonb.

I have a varchar column that contains integer id references to another table, and want to change the column type from varchar to int. I realize varchar -> int is not a safe cast, but all of the data in the table is an integer, just stored as text.

Is there a way to use the change column type recipe for this, or does this need to be a a custom migration?

Hi again!

We just ran into trouble when trying to add a new foreign key to an existing table using add_reference_concurrently. We already had a different foreign key pointing to the same target able (using a different column). This caused online_migrations to refuse to add the new foreign key, as it erroneously believes the foreign key already existed. Apparently it does not take the column into consideration when checking if the foreign key already exists.

Something like this therefore does not work:

add_reference_concurrently :foos, :baz, foreign_key: { to_table: :bars }
add_reference_concurrently :foos, :quux, foreign_key: { to_table: :bars }

Thanks for looking into this!

When rolling back a migration (but not running the migration in the first place) with rails db:rollback:primary, we're getting the following error:

undefined method `__raw_connection' for #<ActiveRecord::Migration::CommandRecorder:0x0000aaaaf43894c0>
db/migrate/20220318210531_add_url_to_user.rb:3:in `change'

The migration file is:

class AddUrlToUser < ActiveRecord::Migration[6.1]
  def change
    add_column(:user, :url, :string)
  end
end

The error is originating from:

More details:

• online_migrations: 0.4.0
• rails: 6.1.4.7
• config/initializers/online_migrations.rb is mostly the defaults, other than setting start_after, small_tables, and lock_timeout_limit.

Any thoughts on what might be wrong before I dive in further? What class is connection typically?

And thanks for the gem! It looks super helpful to make our migrations safer.