-
-
Notifications
You must be signed in to change notification settings - Fork 528
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
Migrations - Add Multiple Columns Example? #133
Comments
I was just looking for the answer to this and couldn't find it anywhere. I managed to figure it out though. 'use strict';
module.exports = {
up: function (queryInterface, Sequelize) {
return [
queryInterface.addColumn(
'users',
'email',
{
type: Sequelize.STRING,
allowNull: false
}
),
queryInterface.addColumn(
'users',
'encryptedPassword',
{
type: Sequelize.STRING,
allowNull: false
}
)
];
},
down: function (queryInterface, Sequelize) {
return [
queryInterface.removeColumn('users', 'email'),
queryInterface.removeColumn('users', 'encryptedPassword')
];
}
}; Regarding the error, I'd need to see your migrations to have a better idea about what is going on. Sequelize will run migrations sorted by the filename. You can run
from the command line to create a new migration prefixed with a timestamp that will order by when they were created. |
Hey guys. The migration functions are supposed to return promises. So if you want to do multiple things in a migration, just return a chained promised like this:
|
as said, we would need to see your migration to answer the question about the error |
@sdepold That was helpful. I saw no mention of these migration functions returning promises. Was running into some weird race conditions due to returning an array of functions that are asynchronous in nature. |
I'd love to add multiple columns in a single migration as well. But chaining the promises actually isn't a good idea. There currently isn't any type of rollback for column changes. So if you try and add 2 columns in the same migration and the 2nd fails. Then the migration will fail but column no. 1 will still exist. And when you re-run the migrations they will eternally fail because the migration will try to create column no. 1 a second time and conflict. Thanks to that, as annoying as it is, you actually need a separate migration for each column you want to add, each index you want to add, etc. For your migrations to be reliable. |
Though I do wonder if there is a nice alternative to unreliable migrations or being forced to use a separate migration file for each individual change in a large migration. Something like a multi-migration, ie: a single migration file that actually contains multiple migrations. "use strict";
module.exports = [
{
up: function(migration, Sequelize) {
return migration
.addColumn(
'FooBar',
'B',
{
type: Sequelize.STRING,
allowNull: true
});
},
down: function(migration) {
return migration
.removeColumn('FooBar', 'B');
}
},
{
up: function(migration, Sequelize) {
return migration
.addColumn(
'FooBar',
'C',
{
type: Sequelize.STRING,
allowNull: true
});
},
down: function(migration) {
return migration
.removeColumn('FooBar', 'C');
}
}
]; For these |
I also find this annoying. +1 for @dantman's suggestion. As a work around I have found that adding the migration name manually to the SequelizeMeta table allows me to run undo, which, if you structure the rollback symmetrically, will then error out at the same point. This will have the effect of removing the same columns up to the point of error, which is effectively a rollback of a multi column update. This work around could be made easier by having the sequelize db:migrate:undo be able to specify a specific migration that needs to be undone, or run it on the one that is not in SequelizeMeta. IE sequelize should anticipate that people may want to rollback their migrations when they fail. |
Hey guys, just thought I would share a solution in case you are still looking for one. I don't like super-chaining my commands because it gets hard to read after a while, and I get lost in the indentation. Especially if you're adding a ton of column changes or other alterations. I generally don't like to chain column changes anyway because if the migration fails then you have a big issue on your hands in terms of figuring out what worked and what didn't. So I basically only use chaining for initial table creation + indexes (or else I would need a separate migration for each index, which is painful). I do this because the indexes are erased when calling down() when the table is dropped. So my migrations would look like this: var promise = require('bluebird');
module.exports = {
up: function (queryInterface, Sequelize) {
var actions = [];
// Create the main table
actions.push({
exec: function() {
return queryInterface.createTable(...);
}
});
// Append the indexes and whatnot
actions.push({ exec: function() { return queryInterface.addIndex('users', ['...']); }});
// Go through one by one (with concurrency!) and process each function
return promise.map(actions, function(item) {
return item.exec();
}, { concurrency: 1 });
},
down: function (queryInterface, Sequelize) {
queryInterface.dropTable('users');
}
}; |
With node 6 the following appears to work...
|
@parris If you add another add/removeColumn to that migration it will suffer from the same issue as chaining using If even one of your addColumn commands succeeds but any others fail, then your migrations will be stuck in a state they cannot migrate or escape from. Any attempt to re-run the migration will fail, because it will try to create a column that already exists. Any attempt to undo the migration will also fail, because it will try to remove a column that doesn't exist. |
@dantman I see what do you mean, how can we protect against that? |
@dantman Maybe just like @jylinman proposed, though in the inverse operation triggers an error in the middle, maybe can get the migrations in a recoverable state if the operations are inversed too? I mean (in the same migration): up:
down:
I'm asking this because I have a migration which change a |
Aren't transactions the solution to the bad state issue described by dantman? #133 (comment) I believe that's how Rails deal with multiple DB changes in a single migration. Each migration runs within a transaction. If any individual change fails, the transaction rolls back. |
@createthis Agree. If transactions could be used in the migrations, that will be the solution for this. |
They... can't? |
@createthis @diosney MySQL (at least) does not support transactions for ALTER TABLE. Any transactions are implicitly committed before table structure changes start. To be honest, I don't blame them. Transactions are primarily meant for rolling back data changes. It sounds absurd to try and attempt to apply them to changes that alter the very structure of that data which can implicitly alter values within every single row of a table of undefined size. |
Thanks. That's interesting. I never noticed that. Worked with Rails and MySQL for years. I guess the recent trend toward Postgres spoiled me. So, in sqlite and postgres, would something like this work? up: function (queryInterface, Sequelize) {
var sequelize = queryInterface.sequelize;
return sequelize.transaction(function (t) {
var migrations = [];
migrations.push(queryInterface.addColumn(
'products',
'some_count',
Sequelize.INTEGER.UNSIGNED,
{transaction: t}
));
migrations.push(queryInterface.addColumn(
'products',
'some_other_property',
Sequelize.INTEGER.UNSIGNED,
{transaction: t}
));
return Promise.all(migrations);
});
},
down: function (queryInterface, Sequelize) {
var sequelize = queryInterface.sequelize;
return sequelize.transaction(function (t) {
var migrations = [];
migrations.push(queryInterface.removeColumn('products', 'some_count', {transaction: t}));
migrations.push(queryInterface.removeColumn('products', 'some_other_property', {transaction: t}));
return Promise.all(migrations);
});
} Seems to work on my end, but I haven't tried to make it fail yet. |
@createthis Transaction wise it should work in PG/SQlite. However I'd advise against an array+Promise.all based method. You're doing 2 ALTER table operations in parallel. You're protected from the worst by transactions, but you could still end up with weird race conditions; like the order of columns in your table being different between different runs and confusing changes in error messages if both your alterations fail but which one errors out first differs between migration attempts. |
@dantman actually, in the previous example I gave, I'm getting this: Sequelize [Node: 6.8.0, CLI: 2.4.0, ORM: 3.24.4, sqlite3: ^3.1.6]
Loaded configuration file "config/config.json".
Using environment "development".
== 20161020154112-AddStatisticsToProducts: reverting =======
Unhandled rejection SequelizeDatabaseError: SQLITE_ERROR: no such table: products I can delete the sqlite db and run the migrations forward, but any time I try to undo the last migration I get the above error. It's like it's grabbing the wrong table or something because I manually verified the table exists using the sqlite3 cli tool. |
Spent some time debugging this. Turns out the error message in the previous comment was a direct result of the queries happening async as @dantman warned. I'd forgotten that SQLite has no drop column. Sequelize simulates drop column by dropping the table and recreating it. In this case it was dropping the table twice, thus the error on the second drop table. I figured it out by enabling sequelize console logging and examining the SQL queries logged. Here's my revised strategy (for posterity): up: function (queryInterface, Sequelize) {
var sequelize = queryInterface.sequelize;
return sequelize.transaction(function (t) {
return queryInterface.addColumn('products', 'some_count', Sequelize.INTEGER.UNSIGNED, {transaction: t})
.then(function () {
return queryInterface.addColumn('products', 'some_other_property', Sequelize.INTEGER.UNSIGNED, {transaction: t});
});
});
},
down: function (queryInterface, Sequelize) {
var sequelize = queryInterface.sequelize;
return sequelize.transaction(function (t) {
return queryInterface.removeColumn('products', 'some_count', {transaction: t})
.then(function () {
return queryInterface.removeColumn('products', 'some_other_property', {transaction: t});
});
});
} I've tested this and it works as expected in multiple migrate/undo cycles. The indentation is a little deeper than I'd like, but at least it never goes deeper no matter how many |
@createthis Guess you discovered another race condition I didn't think about that makes running db migration steps in parallel a bad idea. Btw, if you're willing to use Babel for now, then async/await could flatten that a little more. |
Migrations must be Also having multiple migrations per file is not recommended, even if the UP
DOWN
If on the UP |
@eddiejaoude probably that's why createthis used transactions, if the B will fail then all the migration will rolleback. |
@createthis you might get better indentation by putting the first query into
|
This what worked: sequelize migration:create --name add-password-to-user
And then If not happy sequelize db:migrate:undo Respecting the bellow comment from scottbeacher, the above worked for me on a PostgreSQL database. |
@iamfree-com Alter table causes implicit commits, can't use transactions with those. Has already been mentioned. https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html |
Guys, I'm posting here only to document the solution that I have used to have a more clean code on scenario where we have multiple removeColumn calls. Async/await were a good solution for my case:
|
@godfather ah man I have been searching for about 2 hours for a manageable solution to chaining migrations. This is the best solution I have seen so far. Thank you! |
Note, there appears to be a difference in a couple of the suggestions proposed here. Returning an array of promises might be invoking a race condition. Depending on what you're doing, this may not matter. I didn't get an error but I had an issue where a column rename wasn't working after running changeColumn on the same column. Switching to a |
Just a thought, but if you use await, my understanding is that the function will effectively be suspended until the promise resolves whereby it will continue to the next line of code. So there should never be a race condition as I understand it? |
I'm trying to create tables having dependency in this order in my single migration file but on migration I'm facing the error
|
Hey orton009, I would use multiple migrations for this and not try to create/drop multiple tables in one migration. Single migrations with multiple methods is more like if you are adding a whole bunch of columns to a table. If you really want to do it in one method, you can only have ONE return method per javascript function, because Finally, just a Github tip (and tech websites like this one), you can use three backticks in a row around your code to so that it is readable. Your code above is indecipherable otherwise! Here is how it works: Similarly for single line, you can surround with single backticks: P.S. As far as I know, using one column as the minimum value of another column is not possible. This sort of thing is kept in the code as business logic and beyond the responsibilities of the database.
If you really must do it, it may be possible by using triggers: https://www.siteground.com/kb/mysql-triggers-use/ In any case, this is beyond the scope of this thread - so if you have any more questions regarding how to set minimum values in this way, I would start a new issue thread: https://github.com/sequelize/cli/issues/new |
Can you provide me any resource on how to create multiple migrations in same file and run those migrations in an order in order to maintain table dependencies |
Dude, it's in this very thread :P Scroll up! |
This is giving me the error "replace " not defined
|
I'm gonna guess that there's a race condition because there are references to other tables.
I still highly recommend using multiple migrations, or at least two. However if it must be done in one you can probably put the
|
Its not adding the foreign key error |
If I am using multiple migrations files for multiple tables then how can I control table creation as tables are using foreign key so former tables must be created before them, how can I specify which migrations to take place first and executing dependent migrations later |
I like @iamfree-com's suggestion most, but the next code fails: down: (queryInterface, Sequelize) => {
return queryInterface.sequelize.transaction(function handleTransaction (t) {
return Promise.all([
queryInterface.renameColumn(
'Users',
'uid',
'email',
{transaction: t}
),
queryInterface.removeColumn(
'Users',
'active',
{transaction: t}
),
]);
});
} |
@shepelevstas those should be separate migration steps, if one fails, you won't know which one |
Is this OK? Works for me: down: (queryInterface, Sequelize) => {
return queryInterface.sequelize.transaction(async transaction => {
return [
await queryInterface.renameColumn(
'Users',
'uid',
'email',
{transaction}
),
await queryInterface.removeColumn(
'Users',
'active',
{transaction}
)
]
})
} |
then when you try to rollback or forwards, one of those might have already run successfully and it won't know but when it tries to run it again it will always now fail |
@eddiejaoude No. Transaction makes the whole migration to fail and rollback if any of modifications fail. |
@shepelevstas where did you read that? Its not in the docs. In fact, there is an open issue for that feature, so I dont think it exists yet! But its your product, if you would rather save a few seconds when running the migrations at the cost of it being more fragile, then thats your decision 🤓 . Good luck to you 😉 #1364 |
@eddiejaoude it's one of the main tabs in the docs: http://docs.sequelizejs.com/manual/tutorial/transactions.html |
I didn't see you were using I would like to understand the problem you are trying to solve? |
@shepelevstas Alter table (which is what removeColumn and renameColumn will use) causes implicit commits, transactions don't affect those. The change is committed immediately so there is no way to rollback if a later step in the transaction fails. Has already been mentioned several times in this ticket. https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html |
I tested it in SQLite, and it properly rolls back the whole transaction if one of the later alterations fails. I checked the SQL query, it creates a backup table during transaction. But, yes, it is better to make alterations in a chain, as some of them interfear with each other. Still transaction helps a lot. |
I am still struggling to see the problem you are trying to solve by running multiple migrations in a single migration file? |
Hello .... I encountered the same scenario where I need to add multiple columns in one migration and this works for me.
|
this works for me. |
Create exercism directory on configuration
I am somewhat of a newbie, and I have a single add column working, but a couple of questions.
how can I add multiple columns in one migration file...
also, when I rerun my app I am seeing ER_DUP_FIELDNAME: Duplicate column name 'test' when I run the node server.js the next time...which is correct because it has already been created, should I worry abot catching and ignore the error or just let it run as it is?
Also, if there are multiple migration files in the directory what order are they ran?
The text was updated successfully, but these errors were encountered: