fix: support changing comments in MySQL columns#6903
Merged
imnotjames merged 1 commit intotypeorm:masterfrom Oct 18, 2020
Merged
fix: support changing comments in MySQL columns#6903imnotjames merged 1 commit intotypeorm:masterfrom
imnotjames merged 1 commit intotypeorm:masterfrom
Conversation
imnotjames
force-pushed
the
fix/mysql-column-comments
branch
3 times, most recently
from
bbaa826 to
05aa6d3
Compare
pleerock
reviewed
Oct 15, 2020
Member
|
It's super important for us to avoid breaking changes, and changes in schema sync are the most dangerous ones. That's why I'm always super dangerous on merging such PRs. Please make sure to check schema sync under different circumstances before merging it. |
in the mysql driver, column comments were only half supported - adding them to new columns & new tables was possible, but updating existing columns to include comments was not possible there also was no escaping done on the column comments - so depending on the contents of your comment, an invalid query could be generated adds support to add comments to existing columns, adds simple comment escaping code, & creates tests to verify the behavior
imnotjames
force-pushed
the
fix/mysql-column-comments
branch
from
05aa6d3 to
37b0754
Compare
Contributor
Author
|
Entirely agree. There's a number of edge cases tested here and the only code path that's not being a comments changing test should be safe. I've tested various possible failure states and also changing the comments. There's existing tests that test changing columns without changing comments. Nothing is 100% safe, but these new tests help bring that % up a bit :) |
zaro
pushed a commit
to zaro/typeorm
that referenced
this pull request
Jan 12, 2021
in the mysql driver, column comments were only half supported - adding them to new columns & new tables was possible, but updating existing columns to include comments was not possible there also was no escaping done on the column comments - so depending on the contents of your comment, an invalid query could be generated adds support to add comments to existing columns, adds simple comment escaping code, & creates tests to verify the behavior
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
in the mysql driver, column comments were only half supported -
adding them to new columns & new tables was possible, but updating
existing columns to include comments was not possible
adds support to add comments to existing columns & creates tests
to verify the behavior
Closes #6336