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

TypeORM foreign_key names #2542

Open
HoseinGhanbari opened this issue Nov 23, 2023 · 3 comments
Open

TypeORM foreign_key names #2542

HoseinGhanbari opened this issue Nov 23, 2023 · 3 comments
Assignees
Labels
ORM Issues relating to potential bugs in the ORM type: bug 🐛 Something isn't working
Milestone

Comments

@HoseinGhanbari
Copy link
Contributor

Describe the bug

Hi,

When switching from dev to prod or vice versa, using yarn migration:generate auto-migration-file will almost all of the time produces MANY unnecessary sql statements like the following

ALTER TABLE `XYZ` DROP FOREIGN KEY `FK_000000000000000000000000000` ...
ALTER TABLE `XYZ` ADD CONSTRAINT `FK_111111111111111111111111111` ...

Actually these are just implicit FK names which are gets considered as unsynchronized changes that have no effect on the business/logics that are going to be executed. BUT They are really annoying as you are consistently getting the warning of Your database schema does not match your current configuration which is almost a false alarm.

Also ...

Who knows what's the meaning of PK_0669fe20e252eb692bf4d344975 ?

This is the related github issue on typeorm about using explicitly named FK
typeorm/typeorm#1355

To Reproduce

  1. MySQL version 8.0 is a must to be used as database.
  2. Generate and run migrations on remote server.
  3. Use a cloned version of your remote database as your local database.
  4. Generate and run migrations on local server.
  5. There will be lots of unnecessary sql statements trying to drop and then recreating many FK

Expected behavior
It would be great if we can add support of an explicit naming strategy with no hashed values for (primaryKeyName/uniqueConstraintName/relationConstraintName/foreignKeyName/indexName) to Vendure's internal TypeORM as an option for our users so there will be no false alarm of unsynchronized database schema messages also actual database schema is going to use meaningful names.

Environment:

  • @vendure/core version: 2.1.3
  • Nodejs version: v20.9.0
  • Database@Dev: MySQL - 8.0.34 - Win64 Host
  • Database@Prod: MySQL - 8.0.29 - Linux Host

Additional context
nothing required as an additional context

@HoseinGhanbari
Copy link
Contributor Author

HoseinGhanbari commented Nov 23, 2023

Now that I'm using a custom naming strategy (actually a fixed one based on table, column and field names), the issue still exists with another wired behavior which I think more related to MySQL itself.

Whenever you try generating migration files while the migrations dir is empty, there will be a new file created which is going to delete all FK first then recreate them with the same name but different Referential Actions for ON DELETE AND ON UPDATE.

Here is an example of what were genereted for a single FOREIGN KEY:

migration UP

ALTER TABLE `authentication_method` DROP FOREIGN KEY `FK_authentication_method_userId_user_id`;

ALTER TABLE `authentication_method` ADD CONSTRAINT `FK_authentication_method_userId_user_id` FOREIGN KEY (`userId`) REFERENCES `user`(`id`) ON DELETE NO ACTION ON UPDATE NO ACTION;

migration DOWN

ALTER TABLE `authentication_method` DROP FOREIGN KEY `FK_authentication_method_userId_user_id`;

ALTER TABLE `authentication_method` ADD CONSTRAINT `FK_authentication_method_userId_user_id` FOREIGN KEY (`userId`) REFERENCES `user`(`id`) ON DELETE RESTRICT ON UPDATE RESTRICT;

So it seems TypeORM wants NO ACTION but MySQL prefers RESTRICT 🤯

@HoseinGhanbari
Copy link
Contributor Author

Actually found some related issues on TypeORM's official repository, which were opened 2018 and still receiving comments in 2023.

Someone found the solution to use mariadb type for a mysql instance but it doesn't seem logical.

typeorm/typeorm#1686
typeorm/typeorm#5960

So as @michaelbromley mentioned before the DefaultNamingStrategy of TypeORM doing its job well by generating deterministic names for "identifiers" but the issue is more about MySQL and TypeORM's beahvior about Referential Actions.

@HoseinGhanbari
Copy link
Contributor Author

using the DefaultNamingStrategy will produces the following statements for the sample FK mentioned above.

migration UP

ALTER TABLE `authentication_method` DROP FOREIGN KEY `FK_00cbe87bc0d4e36758d61bd31d6`;
ALTER TABLE `authentication_method` ADD CONSTRAINT `FK_00cbe87bc0d4e36758d61bd31d6` FOREIGN KEY (`userId`) REFERENCES `user`(`id`) ON DELETE NO ACTION ON UPDATE NO ACTION;

migration DOWN

ALTER TABLE `authentication_method` DROP FOREIGN KEY `FK_00cbe87bc0d4e36758d61bd31d6`;
ALTER TABLE `authentication_method` ADD CONSTRAINT `FK_00cbe87bc0d4e36758d61bd31d6` FOREIGN KEY (`userId`) REFERENCES `user`(`id`) ON DELETE RESTRICT ON UPDATE RESTRICT;

@michaelbromley michaelbromley added the ORM Issues relating to potential bugs in the ORM label Jun 17, 2024
@dlhck dlhck added this to the v4.0.0 milestone Sep 27, 2024
@dlhck dlhck moved this to 👀 Under consideration in Vendure OS Roadmap Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ORM Issues relating to potential bugs in the ORM type: bug 🐛 Something isn't working
Projects
Status: 👀 Under consideration
Development

No branches or pull requests

3 participants