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

fix: resolve MySQL unique index check when bigNumberStrings is false #4822

Merged
merged 1 commit into from Oct 18, 2019

Conversation

SteveByerly
Copy link
Contributor

Part of fix for #2737 and #2943

@SteveByerly
Copy link
Contributor Author

Further information:
MySQL indexes have a Non_unique property that normally has a value of either "0" or "1".

When the option for bigNumberStrings is set to false (where big numbers are not coerced into strings), the value for Non_unique comes back as the numeric 0 or 1.

There may be other locations affected by this subtle difference, but I have only encountered it when dealing with migrations. Every new migration attempts to update those columns with a unique index (including dropping and re-adding the index) even though the column definition has not changed.

There is at least one other case that causes unchanged columns to be updated in every migration, but it has a workaround and I would prefer fixing it in a separate PR.

Issue:

  • Numeric columns without a width specified are always recreated
    Source:
  • MysqlDriver.prototype.findChangedColumns compares the width defined in the decorator with the value returned from the database. If no width is provided, the value is undefined and compared to the default value from the DB.

@fw623
Copy link

fw623 commented Sep 27, 2019

I found the same problem with unique Index getting dropped and readded every migration.
My attempt at fixing: PR4819
This should also fix Issue #3841

@@ -1340,7 +1344,7 @@ export class AuroraDataApiQueryRunner extends BaseQueryRunner implements QueryRu
table: table,
name: constraint["INDEX_NAME"],
columnNames: indices.map(i => i["COLUMN_NAME"]),
isUnique: constraint["NON_UNIQUE"] === "0",
isUnique: constraint["NON_UNIQUE"] == "0", // tslint:disable-line:triple-equals - trying to find truthy value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imagine somebody reading this code. Can you please explicitly specify what logic should be in there (as I understood we should check for === "0" && === 0? If so, please change it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pleerock updated the code to be more explicit.

Pulled in latest from master and rebased.

I debated pulling that logic out into a method on the QueryRunner, but didn't want to overstep and make assumptions.
something like:
private isUniqueIndex(constraint): boolean

If you'd rather go that route, I can update. Otherwise let me know if there's anything else you'd like changed.

@pleerock
Copy link
Member

@fw623 correct its a duplicate, but here we also have a test and both drivers covered. We'll need to close your PR once this one is completed

@SteveByerly SteveByerly force-pushed the bugs-2943-mysql-unsigned branch 2 times, most recently from 9050f56 to 47e969a Compare October 18, 2019 16:50
@pleerock
Copy link
Member

I think it looks okay now, thanks

@pleerock pleerock merged commit d205574 into typeorm:master Oct 18, 2019
@SteveByerly SteveByerly deleted the bugs-2943-mysql-unsigned branch August 25, 2020 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants