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

removeColumn tries to delete non-existant foreign key constraint (mysql) #5808

Closed
DeX3 opened this Issue Apr 27, 2016 · 7 comments

Comments

9 participants
@DeX3

DeX3 commented Apr 27, 2016

Hello,

I've noticed the following problem when trying removing a column that has an index other than a foreign key constraint associated with it (in mysql).

What am I doing?

    return migration.createTable( 'Images', {
      id: {
        type: Sequelize.INTEGER.UNSIGNED,
        primaryKey: true,
        autoIncrement: true,
        allowNull: false
      },
      path: {
        type: Sequelize.STRING,
        allowNull: false
      }
    } )
    .then( function() {
      return migration.addIndex( 'Images', [ 'path' ], {
        indexName: 'images_path_unique',
        indicesType: 'UNIQUE'
      } );
    } )
    .then( function() {
      return migration.removeColumn( 'Images', 'path' );
    } );

What do I expect to happen?

The path column should be dropped along with the UNIQUE index.

What is actually happening?

the mysql-specific implementation for removeColumn tries to manually drop foreign key constraints before actually dropping the column. However, the query returned by getForeignKeyQuery lib/dialects/mysql/query-generator.js:352 also seems to return indexes on the relevant column (like UNIQUE). removeColumn then tries to remove that index with DROP FOREIGN KEY which causes an error:

{ [SequelizeDatabaseError: ER_CANT_DROP_FIELD_OR_KEY: Can't DROP 'images_path_unique'; check that column/key exists]
  name: 'SequelizeDatabaseError',
  message: 'ER_CANT_DROP_FIELD_OR_KEY: Can\'t DROP \'images_path_unique\'; check that column/key exists',
  parent:
   { [Error: ER_CANT_DROP_FIELD_OR_KEY: Can't DROP 'images_path_unique'; check that column/key exists]
     code: 'ER_CANT_DROP_FIELD_OR_KEY',
     errno: 1091,
     sqlState: '42000',
     index: 0,
     sql: 'ALTER TABLE `Images` DROP FOREIGN KEY `images_path_unique`;' },
  original:
   { [Error: ER_CANT_DROP_FIELD_OR_KEY: Can't DROP 'images_path_unique'; check that column/key exists]
     code: 'ER_CANT_DROP_FIELD_OR_KEY',
     errno: 1091,
     sqlState: '42000',
     index: 0,
     sql: 'ALTER TABLE `Images` DROP FOREIGN KEY `images_path_unique`;' },
  sql: 'ALTER TABLE `Images` DROP FOREIGN KEY `images_path_unique`;' }

Also, I noticed that the getForeignKeyQuery-function doesn't restrict the query to the relevant database (mysql's INFORMATION_SCHEMA is global).

It seems like mysql is able to remove unique indexes cleanly along with the column when the parent column is dropped. Ideally, the getForeignKeyQuery function should really only return proper foreign key indexes.

Dialect: mysql
Database version: mysql Ver 14.14 Distrib 5.7.11, for osx10.11 (x86_64)
Sequelize version: 3.20.0

@mickhansen mickhansen added the bug label Apr 27, 2016

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Apr 27, 2016

Sounds like you got a decent handle on the cause, a PR would be great.

paolord added a commit to paolord/sequelize that referenced this issue May 1, 2016

@paolord paolord referenced this issue May 1, 2016

Closed

fix attempt on removeColumn issue #5808 #5830

2 of 5 tasks complete

paolord added a commit to paolord/sequelize that referenced this issue May 1, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 2, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 2, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 3, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 3, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 4, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 7, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 7, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 7, 2016

paolord added a commit to paolord/sequelize that referenced this issue May 7, 2016

janmeier added a commit that referenced this issue May 10, 2016

@janmeier janmeier closed this May 10, 2016

@scottbeacher

This comment has been minimized.

scottbeacher commented Sep 2, 2016

I'm still having this issue with version 3.24.1 and mysql. I have a column created with a unique index. Trying to remove it with a migration fails with the error that it cannot drop the field or key and to check that it exists. But the reason is sequelize is calling ALTER TABLE devices DROP FOREIGN KEY deviceToken

The migration:

module.exports = {
  up: function (queryInterface, Sequelize) {
    return queryInterface.removeColumn('devices', 'deviceToken')
  },

  down: function (queryInterface, Sequelize) {
    return queryInterface.addColumn('devices', 'deviceToken',
      {
        type: Sequelize.STRING,
        unique: true
      }
    )
  }
}

I get the same error as reported in the ticket.

@mayask

This comment has been minimized.

mayask commented Oct 7, 2016

@mickhansen Please reopen this ticket.

Also, I noticed that the getForeignKeyQuery-function doesn't restrict the query to the relevant database (mysql's INFORMATION_SCHEMA is global).

I have prod and test databases and rolling out migration on one causes SequelizeDatabaseError: ER_CANT_DROP_FIELD_OR_KEY error on another. This is insane!

@Undeadlol1

This comment has been minimized.

Undeadlol1 commented Jun 20, 2017

@scottbeacher making two independent migrations worked for me.
Remove indexes in first one, remove column in second one.

@rraina

This comment has been minimized.

rraina commented Aug 24, 2017

Still an open issue as of 3.30.4 ORM and 2.7.0 CLI

@Undeadlol1 -- Tried breaking into two migration with no luck..

@myaskevich -- The issue is exactly about the fact that it doesn't localize the change to the specific database running within the same MySQL instance...

@mickhansen -- what's the best way to make a PR here?

@julienvincent

This comment has been minimized.

julienvincent commented Sep 13, 2017

Running into this issue too

@sushantdhiman

This comment has been minimized.

Member

sushantdhiman commented Sep 30, 2017

Fixed 9ca86af

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