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
Conversation
54edcdc
to
a860c7b
Compare
Further information: When the option for 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:
|
@@ -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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
@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 |
9050f56
to
47e969a
Compare
47e969a
to
f45d482
Compare
I think it looks okay now, thanks |
Part of fix for #2737 and #2943