Migration errors from active storage after rails master to rails alpha migration with app:update

[gingermusketeer]

Steps to reproduce

1. Create an app with rails at: 01fd264d0012 (with active storage)
2. Update to rails 7.0.0.alpha2
3. Run migrations

Expected behavior

Migrations should run without a problem or not be present.

Actual behavior

Migrations are present and fail:

$ rails app:update
# ... SNIP
Copied migration 20210916174641_add_service_name_to_active_storage_blobs.active_storage.rb from active_storage
Copied migration 20210916174642_create_active_storage_variant_records.active_storage.rb from active_storage

After this, check Rails upgrade guide at https://guides.rubyonrails.org/upgrading_ruby_on_rails.html for more details about upgrading your app.
$ rails db:migrate                                                                   main ✭ ◼
== 20210916174641 AddServiceNameToActiveStorageBlobs: migrating ===============
-- column_exists?(:active_storage_blobs, :service_name)
   -> 0.0175s
== 20210916174641 AddServiceNameToActiveStorageBlobs: migrated (0.0176s) ======

== 20210916174642 CreateActiveStorageVariantRecords: migrating ================
-- create_table(:active_storage_variant_records, {:id=>:primary_key})
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

PG::DuplicateTable: ERROR:  relation "active_storage_variant_records" already exists
project/db/migrate/20210916174642_create_active_storage_variant_records.active_storage.rb:5:in `change'

Caused by:
ActiveRecord::StatementInvalid: PG::DuplicateTable: ERROR:  relation "active_storage_variant_records" already exists

The migrations generated are using v6 of rails:

class CreateActiveStorageVariantRecords < ActiveRecord::Migration[6.0]
class AddServiceNameToActiveStorageBlobs < ActiveRecord::Migration[6.0]

The migration code failing is:

 create_table :active_storage_variant_records, id: primary_key_type do |t|
      t.belongs_to :blob, null: false, index: false, type: blobs_primary_key_type
      t.string :variation_digest, null: false

      t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true
      t.foreign_key :active_storage_blobs, column: :blob_id
    end

this is already present from a previous migration:

create_table :active_storage_variant_records, id: primary_key_type do |t|
      t.belongs_to :blob, null: false, index: false, type: foreign_key_type
      t.string :variation_digest, null: false

      t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true
      t.foreign_key :active_storage_blobs, column: :blob_id
    end

System configuration

Rails version:
Described above.
Ruby version:
3.0.2

[brenogazzola]

Is the previous migration still in db/migrate? I've just tested this and the files were only recreated if I had deleted the originals before running app:update

[ghiculescu]

Deleting migration files after they run is a common practice, so maybe we should fix the migrations to be safely runnable many times.

[brenogazzola]

I gave some thought about that idea. It is feasible (ensuring the migrations are reversible would help), but for that to work Rails would have to assume that the database it has access to is at parity with the production database.

[ghiculescu]

I was thinking the migration checks table_exists?

[brenogazzola]

Sorry, I misread your comment as Rails checking if the table exists during app:update and NOT creating the migration at all. I blame it on lack of coffee at the time 😅

[brenogazzola]

By the way. If adding the check is really the solution we are going with, this would be worthy of tagging as “good first issue” 😀

[gingermusketeer]

Using the table_exists? check would mean that it is consistent with the other non failing migration that is generated. I would be happy to put up a PR to address this if you like?

[ghiculescu]

Yes please!

[gingermusketeer]

Put up PR #43260. Let me know what you think!