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

sync() with new alter mode fails on second run when using CONSTRAINTS #7606

Open
skn3 opened this issue May 4, 2017 · 44 comments
Open

sync() with new alter mode fails on second run when using CONSTRAINTS #7606

skn3 opened this issue May 4, 2017 · 44 comments

Comments

@skn3
Copy link

skn3 commented May 4, 2017

What you are doing?

'use strict';

const Sequelize = require('sequelize');

//first connect to the database
const database = new Sequelize('database', 'user', 'password', {
    dialect: 'postgres',
    host: '127.0.0.1',
    port: 5432,

    pool: {
        max: 5,
        min: 0,
        idle: 10000
    },
});

return database.authenticate()
.then(() => {
    //success connecting,
    console.log('database connected');
    return database;
})
.then(database => {
    const containerModel = database.define('TestContainer', {
        id: {
            type: Sequelize.DataTypes.UUID,
            defaultValue: Sequelize.DataTypes.UUIDV4,
            primaryKey: true,
            allowNull: false
        },
    });

    const itemModel = database.define('TestItem', {
        id: {
            type: Sequelize.DataTypes.UUID,
            defaultValue: Sequelize.DataTypes.UUIDV4,
            primaryKey: true,
            allowNull: false
        },
    });

    const userModel = database.define('TestUser', {
        id: {
            type: Sequelize.DataTypes.UUID,
            defaultValue: Sequelize.DataTypes.UUIDV4,
            primaryKey: true,
            allowNull: false
        },
    });

    containerModel.belongsTo(userModel);
    itemModel.belongsTo(userModel);
    containerModel.belongsToMany(itemModel, {through: 'TestContainerItems', timestamps: false, onDelete: 'CASCADE'});

    return database.sync({
        alter: true,
        force: false,
    });
})
.catch(err => {
    //failed to connect
    console.error(err);
    process.exit(1);
});

What do you expect to happen?

Being able to run the example multiple times without fail

What is actually happening?

On second run, an error occurs:

SequelizeDatabaseError: constraint "TestUserId_foreign_idx" for relation "TestContainers" already exists

Dialect: postgres
Database version: 9.6.2
Sequelize version: git master

@skn3
Copy link
Author

skn3 commented May 4, 2017

No idea if this is safe, but needed a quick fix for testing:

function dropForeignKeyConstraints(database) {
    //this is a hack for dev only!
    //todo: check status of posted github issue, https://github.com/sequelize/sequelize/issues/7606
    const queryInterface = database.getQueryInterface();
    return queryInterface.showAllTables()
    .then(tableNames => {
        return Promise.all(tableNames.map(tableName => {
            return queryInterface.showConstraint(tableName)
            .then(constraints => {
                return Promise.all(constraints.map(constraint => {
                    if (constraint.constraintType === 'FOREIGN KEY') {
                        return queryInterface.removeConstraint(tableName, constraint.constraintName);
                    }
                }));
            });
        }));
    })
    .then(() => database);
}

Run before sync to drop all foreign keys. BRUTAL!

@skn3
Copy link
Author

skn3 commented Jul 9, 2017

also related to #7915 and #7649

@demian85
Copy link

demian85 commented Aug 8, 2017

Any news on this?

@joncorrin
Copy link

Any updates on this?

@robin-garnham
Copy link

This is a bit of a pain... is anyone working on a fix?

@icarusmiles
Copy link

Any news on this?

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 22, 2017

@the solution would be to check for existing constraint as a method exists to do it.
As a quick fix but not a long term solution, I've written the following code in the query-interface file in the changeColumn method to check if a constraint already exists and it worked pretty well (starting line 556):

if (this.sequelize.options.dialect === 'sqlite') {
    // sqlite needs some special treatment as it cannot change a column
    return SQLiteQueryInterface.changeColumn.call(this, tableName, attributes, options);
} else {
    /************************************************************
    * HERE IS WHERE I CHECK FOR AN EXISTING CONSTRAINT
    ************************************************************/
    if (attributes[attributeName].references) {
        this.showConstraint(tableName, `${tableName}_${attributeName}_foreign_idx`).then((result) => {
            if (result.length === 0) {
                const query = this.QueryGenerator.attributesToSQL(attributes);
                const sql = this.QueryGenerator.changeColumnQuery(tableName, query);

                return this.sequelize.query(sql, options);
            }
        });
    } else {
        const query = this.QueryGenerator.attributesToSQL(attributes);
        const sql = this.QueryGenerator.changeColumnQuery(tableName, query);

        return this.sequelize.query(sql, options);
    }
}

It is not a long term solution as some code has been duplicated.
I hope it helps

@joncorrin
Copy link

@cyrille-gaucher I added the code written but it's still giving me the SequelizeDatabaseError: relation "email_unique_idx" already exists error. I placed a few console.log() 's to see what was being called and it seems that its not reaching the if conditional and only reaching the else which is the original code if no constraint already exists which it does. Here is the code calling alter models.sequelize.sync({alter: true}).then(function() { server.listen(port, function() { console.log('Express server listening on port ' + server.address().port); }); server.on('error', onError); server.on('listening', onListening); });

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 23, 2017

@joncorrin You're right. This fix is only checking foreign key constraints. It needs to be improved to get it work for any constraint.

@joncorrin
Copy link

@cyrille-gaucher of course. I see now. Any idea on how I could get it to check for unique constraints?

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 23, 2017

@joncorrin Could you provide an model example which fails please? Also, which version of sequelize are you using?

@joncorrin
Copy link

Yes, here is my user model which has a unique constraint on the email attribute. It's causing the error anytime I had a new model with a relation to user or add to the user model.

"use strict";

module.exports = function(sequelize, DataTypes) {
  var User = sequelize.define("User", {
    email: {
      type: DataTypes.STRING,
      unique: true,
      isLowerCase: true,
      allowNull: false
    },
    password: {
      type: DataTypes.STRING,
      allowNull: true
    },
    image: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    name: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    provider:  {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: 'rent'
    },
    uid: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    street1: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    street2: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    city: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    state: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    zip: {
      type: DataTypes.STRING,
      allowNull: true,
      defaultValue: null
    },
    contracts: {
      type: DataTypes.ARRAY(DataTypes.STRING),
      allowNull: true,
      defaultValue: []
    },
    ownerContractSigned: {
      type: DataTypes.BOOLEAN,
      defaultValue: true
    },
    renterContractSigned: {
      type: DataTypes.BOOLEAN,
      defaultValue: true
    },
    customerId: DataTypes.STRING,
    cardId: DataTypes.STRING,
    cardLast4: DataTypes.STRING,
    stripe_user_id: {
      type: DataTypes.STRING,
      defaultValue: null
    },
    needsToRate: {
      type: DataTypes.INTEGER,
      defaultValue: 0
    },
    userName: {
      type: DataTypes.STRING,
      unique: true,
      isLowerCase: true,
      allowNull: true
    },
    finishedTutorial: {
      type: DataTypes.BOOLEAN,
      defaultValue: false
    },
    hasANotification: {
      type: DataTypes.BOOLEAN,
      defaultValue: false
    },
    hasAMessage: {
      type: DataTypes.BOOLEAN,
      defaultValue: false
    },
    referralCode: {
      type: DataTypes.STRING,
      defaultValue: null
    },
    referralBonuses: {
      type: DataTypes.INTEGER,
      defaultValue: 0
    },
    videosRankedIds: {
      type: DataTypes.ARRAY(DataTypes.INTEGER),
      defaultValue: []
    }
  });
  User.associate = function(models) {
    User.hasMany(models.Item, {
      as: 'owner',
      onDelete: "CASCADE"
    });
    User.hasMany(models.Item, {
      as: 'renter',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }
    });
    User.hasMany(models.Message, {
      as: 'messages',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }
    });
    User.hasMany(models.Notification, {
      as: 'notifications',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }
    });
    User.hasMany(models.Transaction, {
      as: 'transactions',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }
    });
    User.hasMany(models.Review, {
      as: 'reviewee',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }

    });
    User.hasMany(models.Review, {
      as: 'reviewer',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }
    });
    User.hasMany(models.Tracker, {
      as: 'tracker',
      onDelete: "CASCADE",
      foreignKey: {
        allowNull: true
      }
    });
    User.hasMany(models.Video, {
      as: 'videos',
      onDelete: "CASCADE",
      foreignKey: 'instructorId',
      constraints: false
    });
  };
  return User;
};

@joncorrin
Copy link

@cyrille-gaucher ^^^

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 23, 2017

@joncorrin I could not reproduce the error in your use case, I'm sorry. Maybe we are not using the same SQL engine (I'm using mysql). What are you using?

@joncorrin
Copy link

joncorrin commented Oct 23, 2017

Ah yes, @cyrille-gaucher , I am using Postgresql. This has been a reocurring issue for me anytime I want to use .sync() to alter a table related to or of the User Model. I appreciate your help and time!

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 23, 2017

Each query-generator needs to implement its own check on constraints.

@icarusmiles
Copy link

icarusmiles commented Oct 23, 2017

@joncorrin Same issue and I am using MySQL, I have a lot of sync issues. However my structure isn't as complex as yours.

@skn3 development fix has temporarily solved most of my issues, have you tried his dropForeignKeyConstraints function?

@joncorrin
Copy link

@miles-collier I haven't. He suggested not to use in production and I'll need to do altering in prod for the most part. I'm sticking to migrations unless someone is able to help out or this gets a fix. I'm in no place to implement this myself.

@icarusmiles
Copy link

icarusmiles commented Oct 23, 2017

@joncorrin Automatic alterations to production aren't recommended anyways.

from the docs:
Migrations are still preferred and should be used in production.

Lots of other sources claim heavy risk for using these alterations, it's still in it's infancy.

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 23, 2017

Maybe there's something a bit less brutal to do. Ignoring what I've written above, the following would work for all dialect I think (still starting line 556):

if (this.sequelize.options.dialect === 'sqlite') {
  // sqlite needs some special treatment as it cannot change a column
  return SQLiteQueryInterface.changeColumn.call(this, tableName, attributes, options);
} else {
  this.showConstraint(tableName).then((results) => {
    const query = this.QueryGenerator.attributesToSQL(attributes);
    const sql = this.QueryGenerator.changeColumnQuery(tableName, query);
    const constraintName = this.QueryGenerator.generateConstraintName(tableName, query[attributeName], attributeName);

    if (constraintName) {
      // Constraint index
      const index = results.findIndex(result => result.constraintName === constraintName);
      if (index !== -1) {
        return;
      }
    }

    return this.sequelize.query(sql, options);
  });
}

Then each query-generator just need to implement a generateConstraintName function which returns null if there is no constraint, or the constraint name otherwise.

MYSQL (tested) / MSSQL (not tested):

generateConstraintName(tableName, query, attributeName) {
  if (query.match(/REFERENCES/)) {
    return tableName + '_' + attributeName + '_foreign_idx';
  }

  return null;
}

POSTGRESQL (not tested):

generateConstraintName(tableName, query, attributeName) {
  if (query.match(/UNIQUE;*$/)) {
    return attributeName + '_unique_idx'
  }
  if (query.match(/REFERENCES/)) {
    return attributeName + '_foreign_idx'
  }
}

@joncorrin
Copy link

joncorrin commented Oct 23, 2017

Unhandled rejection ReferenceError: definition is not defined
    at Object.generateConstraintName (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/sequelize/lib/dialects/postgres/query-generator.js:901:9)
    at showConstraint.then (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/sequelize/lib/query-interface.js:563:52)
    at tryCatcher (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/promise.js:693:18)
    at Async._drainQueue (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/async.js:133:16)
    at Async._drainQueues (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/bluebird/js/release/async.js:17:14)
    at ZoneDelegate.invokeTask (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/zone.js/dist/zone-node.js:425:31)
    at Zone.runTask (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/zone.js/dist/zone-node.js:192:47)
    at ZoneTask.invokeTask (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/zone.js/dist/zone-node.js:499:34)
    at Immediate.ZoneTask.invoke (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/zone.js/dist/zone-node.js:488:48)
    at Immediate.timer (/Users/jonathancorrin/Desktop/workspace/rent-app/node_modules/zone.js/dist/zone-node.js:1909:29)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

It worked kind of. I received this error multiple times before the server started up. But checking my db it seemed to have worked anyways. Did you mean to use query.match(/REFERENCES/)) ?? @cyrille-gaucher

@cyrille-gaucher
Copy link

cyrille-gaucher commented Oct 24, 2017

@joncorrin Yes, my bad. Updated!

@techsin
Copy link

techsin commented Nov 10, 2017

is there solution to this yet

@gmanolache
Copy link

+1

@lext-7
Copy link
Contributor

lext-7 commented Dec 17, 2017

#8795

@sushantdhiman
Copy link
Contributor

Fixed by bb2d0bd

@jaydp17
Copy link

jaydp17 commented Feb 9, 2018

@sushantdhiman I don't think it works for unique constraints 😕

@dlarr
Copy link

dlarr commented Feb 23, 2018

Hello

Could you reopen this, I am using sequelize version 4.33.4, and I still have the issue.

'use strict';
module.exports = (sequelize, DataTypes) => {
   var Task = sequelize.define('Task', {
       title: DataTypes.STRING,
   });

   Task.associate = function (models) {
       models.Task.belongsTo(models.User, {
           onDelete: "CASCADE",
           foreignKey: {
               allowNull: false
           }
       });
   };

   return Task;
};
'use strict';
module.exports = (sequelize, DataTypes) => {
    var User = sequelize.define('User', {
        username: DataTypes.STRING,
        email: DataTypes.STRING
    });

    User.associate = function(models) {
        models.User.hasMany(models.Task);
    };

    return User;
};
Unhandled rejection SequelizeDatabaseError: Can't create table `test_referentiel`.`#sql-e40_b3` (errno: 121 "Duplicate key on write or update")
    at Query.formatError (C:\_dev\test\referentiel\node_modules\sequelize\lib\dialects\mysql\query.js:247:16)
    at Query.handler [as onResult] (C:\_dev\test\referentiel\node_modules\sequelize\lib\dialects\mysql\query.js:68:23)
    at Query.Command.execute (C:\_dev\test\referentiel\node_modules\mysql2\lib\commands\command.js:30:12)
    at Connection.handlePacket (C:\_dev\test\referentiel\node_modules\mysql2\lib\connection.js:502:28)
    at PacketParser.onPacket (C:\_dev\test\referentiel\node_modules\mysql2\lib\connection.js:81:16)
    at PacketParser.executeStart (C:\_dev\test\referentiel\node_modules\mysql2\lib\packet_parser.js:77:14)
    at Socket.<anonymous> (C:\_dev\test\referentiel\node_modules\mysql2\lib\connection.js:89:29)
    at emitOne (events.js:115:13)
    at Socket.emit (events.js:210:7)
    at addChunk (_stream_readable.js:252:12)
    at readableAddChunk (_stream_readable.js:239:11)
    at Socket.Readable.push (_stream_readable.js:197:10)
    at TCP.onread (net.js:589:20)

@techsin
Copy link

techsin commented Mar 23, 2018

just a note, you also encounter this error if sync is being called twice. In my project it was called in index.js in models and then in app.js

@IzaGz
Copy link

IzaGz commented Apr 27, 2018

why is it closed? how to fix it? i tried on sequelize 3.32.1 , 3.27 - nothing works! =(

@radcapitalist
Copy link

I think the issue is still open. I have long since given up on alter mode, though, and just adopted migrations. Migrations are the only way to go once you are in production, IMHO.

@gmanolache
Copy link

@radcapitalist I run alter as a step before deployment and it was fixed for me with this ticket.
On the other hand I don't hava complex stuff.

@tomascharad
Copy link

+1

@gideoncatz
Copy link

+1

1 similar comment
@iwaduarte
Copy link

+1

@Mercuso
Copy link

Mercuso commented Dec 13, 2018

Workaround that I found:
Move all the unique indexes definitions to second argument of define method

const Voucher = sequelize.define('Voucher', {
    code: {
      type: DataTypes.STRING
    },
    receiver_name: {
      type: DataTypes.STRING,
    },
  }, {
    indexes: [
      {
        unique: true,
        fields: ['code']
      }
    ]
  });

Don't know why, but it works with sync({alter: true})
(still doesn't work with unique:true property defined directly at column definition)
checked with Sequelize version 4.37.8 and 4.42.0

@FatalCatharsis
Copy link

+1

@Zummek
Copy link

Zummek commented Sep 30, 2019

I had a similar problem. My solution was setting names to index. Probably the autogenerated name was too long. An autogenerated name is built from fields's names. After setting the name all work fine

indexes: [
    {
        fields: ['barCode'],
        unique: true
    },
    {
        fields: [
    	    'productGroupId',
    	    'attributeValue_1',
    	    'attributeValue_2',
    	    'attributeValue_3'
        ],
        unique: true,
        name: 'products_product_group_id_attribute_values'
    }
];

@danielcl
Copy link

danielcl commented Oct 4, 2019

Yes, thank you! My index name was exactly 1 character too long. I wondered why the error message cut of the last character. Now i know why.
But it think this is a bug of sequelize.

@papb
Copy link
Member

papb commented Oct 6, 2019

Hello everyone, I would like to ask a favor - This issue is old and has a lot of discussion and clutter, can someone please open a new issue summarizing the problem that still exists, providing an SSCCE, and linking to this issue? This will let me and others tackle the issue further.

@papb papb added the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Oct 6, 2019
@TheProgrammer21
Copy link

Still experiencing this issue - A fix would be really important!

@Hacktix
Copy link

Hacktix commented Aug 6, 2020

just a note, you also encounter this error if sync is being called twice. In my project it was called in index.js in models and then in app.js

Can confirm that this still causes the issue.

@amaanrajput
Copy link

I already had a table and I used alter so that the data is not gone. Then I created a second model and when I synced sequelize only picked the previous model. The second model was ignored. I changed it to force but still ignores the second model.

@github-actions github-actions bot removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Jul 25, 2021
@chiragsoni81245
Copy link

Workaround that I found: Move all the unique indexes definitions to second argument of define method

const Voucher = sequelize.define('Voucher', {
    code: {
      type: DataTypes.STRING
    },
    receiver_name: {
      type: DataTypes.STRING,
    },
  }, {
    indexes: [
      {
        unique: true,
        fields: ['code']
      }
    ]
  });

Don't know why, but it works with sync({alter: true}) (still doesn't work with unique:true property defined directly at column definition) checked with Sequelize version 4.37.8 and 4.42.0

This Works for me 🙌

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

No branches or pull requests