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 migrations with polymorphic references create indexes with different name #41693

Closed
ClearlyClaire opened this issue Mar 17, 2021 · 3 comments · Fixed by #42350
Closed

Comments

@ClearlyClaire
Copy link
Contributor

Steps to reproduce

Create a table with a polymorphic references in a < 6.0 migration, run the migration with ActiveRecord 6.

This was most probably introduced by e8437a6.

Minimal testcase
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

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

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "6.1.3"
  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)

class CreateSettings < ActiveRecord::Migration[5.0]
  def self.up
    create_table :settings do |t|
      t.references :target, null: false, polymorphic: true
    end
  end
end

class BugTest < Minitest::Test
  def test_migration_up
    CreateSettings.migrate(:up)
    assert CreateSettings.index_name_exists?(:settings, :index_settings_on_target_type_and_target_id)
  end
end

Expected behavior

The created index should have the same name when the migration runs in ActiveRecord 6 than it had in ActiveRecord 5, in this case, index_settings_on_target_type_and_target_id.

Actual behavior

The migration creates an index with a different name: index_settings_on_target.

System configuration

Rails version: 6.1.3

Ruby version: 2.7.2p137

Gargron pushed a commit to mastodon/mastodon that referenced this issue Mar 19, 2021
* Use ActiveRecord::Result#to_ary instead of deprecated to_hash

They do the same thing, and to_hash has been removed from Rails 6.1

* Explicitly name polymorphic indexes to workaround a bug in Rails 6.1

cf. rails/rails#41693

* Fix incorrect usage of “foreign_key” in migration script

* Use `ActiveModel::Errors#delete` instead of deprecated clear method

* Fix link headers tests on Rails 6.1

Rails 6.1 adds values to the Link header by default, thus it is not a
LinkHeader object anymore. Fix the test to parse the Link header instead
of assuming it is a LinkHeader.
@aglushkov
Copy link
Contributor

Same issue #41547

@zzak
Copy link
Member

zzak commented Jun 2, 2021

@ClearlyClaire @aglushkov Could you give #42350 a try and let us know if that solves your issue?

@rails-bot
Copy link

rails-bot bot commented Aug 31, 2021

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Aug 31, 2021
@rails-bot rails-bot bot closed this as completed Sep 7, 2021
chrisguida pushed a commit to Start9Labs/mastodon that referenced this issue Feb 26, 2022
* Use ActiveRecord::Result#to_ary instead of deprecated to_hash

They do the same thing, and to_hash has been removed from Rails 6.1

* Explicitly name polymorphic indexes to workaround a bug in Rails 6.1

cf. rails/rails#41693

* Fix incorrect usage of “foreign_key” in migration script

* Use `ActiveModel::Errors#delete` instead of deprecated clear method

* Fix link headers tests on Rails 6.1

Rails 6.1 adds values to the Link header by default, thus it is not a
LinkHeader object anymore. Fix the test to parse the Link header instead
of assuming it is a LinkHeader.
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.

4 participants