Invalid migration when renaming more than one column

[merceyz]

Describe the bug

Renaming multiple columns and then running a migration tries to rename the first column for all renames

Stack trace

(node:11940) UnhandledPromiseRejectionWarning: InvalidFieldNameException: alter table `user` rename column `created` to `updated_at`; - SQLITE_ERROR: no such column: "created"
    at SqliteExceptionConverter.convertException (C:\mikro-repro\.yarn\$$virtual\@mikro-orm-sqlite-virtual-21fed487d3\0\cache\@mikro-orm-sqlite-npm-4.3.5-dev.38-d8250c5121-a32db6e214.zip\node_modules\@mikro-orm\sqlite\dist\SqliteExceptionConverter.js:32:20)
    at SqliteDriver.convertException (C:\mikro-repro\.yarn\$$virtual\@mikro-orm-core-virtual-2533f208ba\0\cache\@mikro-orm-core-npm-4.3.5-dev.38-2275eb37e3-078a472359.zip\node_modules\@mikro-orm\core\dist\drivers\DatabaseDriver.js:171:54)
    at C:\mikro-repro\.yarn\$$virtual\@mikro-orm-core-virtual-2533f208ba\0\cache\@mikro-orm-core-npm-4.3.5-dev.38-2275eb37e3-078a472359.zip\node_modules\@mikro-orm\core\dist\drivers\DatabaseDriver.js:175:24
    at Function.runSerial (C:\mikro-repro\.yarn\$$virtual\@mikro-orm-core-virtual-2533f208ba\0\cache\@mikro-orm-core-npm-4.3.5-dev.38-2275eb37e3-078a472359.zip\node_modules\@mikro-orm\core\dist\utils\Utils.js:463:22)
    at C:\mikro-repro\.yarn\$$virtual\@mikro-orm-migrations-virtual-6febcf263c\0\cache\@mikro-orm-migrations-npm-4.3.5-dev.38-46d36039b4-fdc438f289.zip\node_modules\@mikro-orm\migrations\dist\MigrationRunner.js:23:17
    at SqliteConnection.transactional (C:\mikro-repro\.yarn\$$virtual\@mikro-orm-knex-virtual-8f51296535\0\cache\@mikro-orm-knex-npm-4.3.5-dev.38-ae8f580068-44a786729d.zip\node_modules\@mikro-orm\knex\dist\AbstractSqlConnection.js:53:25)
    at MigrationRunner.run (C:\mikro-repro\.yarn\$$virtual\@mikro-orm-migrations-virtual-6febcf263c\0\cache\@mikro-orm-migrations-npm-4.3.5-dev.38-46d36039b4-fdc438f289.zip\node_modules\@mikro-orm\migrations\dist\MigrationRunner.js:20:13)

To Reproduce

Repro and stacktrace uses sqlite but happens for mysql as well

import 'reflect-metadata';
import { Entity, MikroORM, PrimaryKey, Property } from '@mikro-orm/core';

async function createAndRunMigration(entities: any[]) {
	const db = await MikroORM.init({
		type: 'sqlite',
		entities: entities,
		dbName: __dirname + '/db.sqlite',
		migrations: {
			path: __dirname + '/migrations',
		},
	});

	await db.getMigrator().createMigration();
	await db.getMigrator().up();

	await db.close();
}

@Entity({ tableName: 'user' })
class UserBefore {
	@PrimaryKey()
	id: string;

	@Property()
	created = new Date();

	@Property()
	updated = new Date();

	@Property()
	deleted = new Date();
}

@Entity({ tableName: 'user' })
class UserAfter {
	@PrimaryKey()
	id: string;

	@Property()
	createdAt = new Date();

	@Property()
	updatedAt = new Date();

	@Property()
	deletedAt = new Date();
}

(async () => {
	await createAndRunMigration([UserBefore]);

	await new Promise((resolve) => {
		setTimeout(resolve, 2000);
	});

	await createAndRunMigration([UserAfter]);
})();

Expected behavior

A migration containing

this.addSql('alter table `user` rename column `created` to `created_at`;');
this.addSql('alter table `user` rename column `updated` to `updated_at`;');
this.addSql('alter table `user` rename column `deleted` to `deleted_at`;');

Additional context

The migration created

this.addSql('alter table `user` rename column `created` to `created_at`;');
this.addSql('alter table `user` rename column `created` to `updated_at`;');
this.addSql('alter table `user` rename column `created` to `deleted_at`;');

Versions

Dependency	Version
node	v14.15.3
typescript	4.1.3
@mikro-orm/core	4.3.4
@mikro-orm/migrations	4.3.4
@mikro-orm/mysql	4.3.4

[B4nan]

Renaming columns is quite magic, it kinda works for single column, but renaming multiple columns with same type/nullability - how can I know what should be renamed to what?

Sure this particular outcome is fixable, but the root cause is what I just noted - it is impossible to tell what old name should be paired with the new name. In other words, it can happen that created will be renamed to updated_at and there is not much we can do about that.

[merceyz]

Yeah, it's quite magical, perhaps the "fix" could be to make it throw when creating the migration instead of when it's executed

[B4nan]

I don't think we wanna throw, but a warning when we create such migration could be enough. And ofc fixing this edge case so we never try to rename multiple columns to one name - that way there will still be a chance that we guessed it correctly (in the end, we process properties in the order they are defined, so for renaming the order should be still same).

[productdevbook]

Some problem how to fixed ? v5 version