Fix unique index created twice #7718
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request addresses an issue with duplicate unique index creation in TypeORM by modifying the handling of unique constraints in composite columns and workspace migrations.
- Modified
CompositeColumnActionFactoryto correctly setisUniqueproperty for composite field columns based on bothalteredFieldMetadata.isUniqueandalteredProperty.isIncludedInUniqueConstraint - Commented out
isUniqueproperty inWorkspaceMigrationRunnerServicewhen creating columns to prevent redundant index creation - These changes aim to resolve the problem of double index creation while maintaining existing functionality for unique constraints
- The modifications may have implications on existing migrations and unique constraint management throughout the system
- Future consideration: potentially moving unique constraint handling to the field level in a later step
2 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
Comment on lines
+186
to
190
| isUnique: | ||
| (alteredFieldMetadata.isUnique && | ||
| alteredProperty.isIncludedInUniqueConstraint) ?? | ||
| false, | ||
| defaultValue: serializedDefaultValue, |
There was a problem hiding this comment.
logic: This change ensures that isUnique is only set when both the field is unique and the specific property is included in the unique constraint. Make sure this logic is consistent with the intended behavior for composite fields.
|
I think you can put back the index decorator in person entity? 🙂 @FelixMalfait |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
isUniquewas passed to TypeORM's column creation API resulting in double index creation because it's already done via the decorator and then inWorkspaceMigrationIndexFactoryIt would be interesting to move it at this field level in a later step, which is why I also fixed
CompositeColumnActionFactoryto pass isUnique on the correct columns, even though it's being ignored later on