Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveRecord::Migration[7.0]'s rename_table uses 7.1's new truncated index name format #50833

Closed
r7kamura opened this issue Jan 22, 2024 · 1 comment · Fixed by #50837
Closed

Comments

@r7kamura
Copy link
Contributor

r7kamura commented Jan 22, 2024

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", "7.1.3"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"

  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_index_name_after_rename_table_with_long_table_name
    long_table_name = "a" * 48

    migration_class = Class.new(ActiveRecord::Migration[7.0]) do
      define_method :migrate do |x|
        create_table :posts do |t|
          t.string :title
          t.index :title
        end
        rename_table :posts, long_table_name
      end
    end
    migration = migration_class.new

    ActiveRecord::Migrator.new(
      :up,
      [migration],
      ActiveRecord::Base.connection.schema_migration,
      ActiveRecord::Base.connection.internal_metadata
    ).migrate

    assert_equal "index_#{long_table_name}_on_title", ActiveRecord::Base.connection.indexes(long_table_name).first.name
  end
end

Expected behavior

The above test should pass.

This means that since the index name was renamed to index_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_on_title in Rails 7.0 and earlier, if ActiveRecord::Migration[7.0] or earlier is specified, the same name should be used even in Rails 7.1 or later.

Actual behavior

The above test fails with the following output:

F

Failure:
BugTest#test_index_name_after_rename_table_with_long_table_name [example.rb:53]:
--- expected
+++ actual
@@ -1 +1 @@
-"index_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_on_title"
+"idx_on_title_0b3d36c24b"



bin/rails test example.rb:32



Finished in 0.026827s, 37.2753 runs/s, 37.2753 assertions/s.
1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

As a result, if db/migrate contains rename_table, the index names created between Rails 7.0 and 7.1 may be different.

System configuration

Rails version: 7.1.3

Ruby version: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]

Other information

This is due to the following change:

I think a similar change to the following is needed for #rename_table:

@r7kamura r7kamura changed the title ActiveRecord::MIgration[7.0]'s rename_table uses 7.1's new truncated index name format ActiveRecord::Migration[7.0]'s rename_table uses 7.1's new truncated index name format Jan 22, 2024
@fatkodima
Copy link
Member

Looking into it. Will open a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants