Renaming a table removes and re-adds FK constraints with no option to disable

[leejarvis]

Hi,

My app has a very large table (lets call it articles) and another slightly smaller table called comments. I added a migration to rename the comments table to messages and was very surprised to see our app blow up when the migration started hanging. After some research it appeared that a constraint was being removed and re-added here. I understand the constraint is being removed and re-added because it cannot be renamed in all cases, but I would expect there to be some way to disable this. Since the following migration:

def change
  rename_table :comments, :messages
end

Appears to have no side affects. Changing the name of the constraints isn't a necessity so it would be nice to turn this off.

Also, in PostgreSQL versions later than 9.2, it's possible to rename a constraint:

alter table messages rename constraint fk_comments_article_id to fk_messages_article_id

Is it possible to have a configuration option to enable constraint renaming or at the very least, disable removing and re-creating these constraints?

[ronen]

Yes, you're right, using PostgreSQL's alter table... rename constraint command would make sense when available, and otherwise renaming should be optional.

This wouldn't be too hard to do. I don't have time right now to properly i.e. add specs, implement, and document it, but I can sketch out how I think it would need to be done. Would you be willing to do a PR?

So here's the sketch...

I'd say change the code you identified here to something along the lines of:

      module RenameTable
        def after(env)
          return unless env.options.fetch(:rename_foreign_keys, true)
          env.connection.foreign_keys(env.new_name).each do | fk |
             begin
               env.connection.rename_foreign_key env.new_name, fk, fk.name.sub(/#{env.table_name}/, env.new_name)
            rescue NotImplementedError
              # sqlite3 can't rename foreign keys, so just skip it
            end
          end
        end
      end

so this should support

  rename_table :comments, :messages, rename_foreign_keys: true   # this is the backwards-compatible default
  rename_table :comments, :messages, rename_foreign_keys: false

And in each of lib/schema_plus/foreign_keys/active_record/connection_adapters/{postgresql,mysql,sqlite3}_adapter.rb add the necessary method

def rename_foreign_key(table_name, fk, new_fk_name)
     ...
end

to do the best it can. I.e. in mysql_adapter.rb it'd do:

  remove_foreign_key(table_name, name: fk.name)
  add_foreign_key(table_name, fk.to_table, :column => fk.column, :primary_key => fk.primary_key, :name => new_fk_name, :on_update => fk.on_update, :on_delete => fk.on_delete, :deferrable => fk.deferrable)

In postgresql_adapter.rb it'd try

  alter table `#{table_name}` rename constraint `#{fk.name}` to `#{new_fk_name}`

and fall back to remove and re-add if that's not supported. And in sqlite3 it'd just

  raise NotImplementedError

What do you think?

[leejarvis]

Odd I thought I'd replied to this.

Thanks for the suggestions, I think your idea is fine. I was hoping to be able to work on this later in the week but unfortunately won't have time until probably after the holidays.

[ronen]

@leejarvis no problem, i'm likewise not gonna be looking at things til after the holidays. happy holidays!