feat: adding column comments for postgres#4461
feat: adding column comments for postgres#4461nionoku wants to merge 2 commits intotypeorm:masterfrom nionoku:master
Conversation
nionoku
changed the title
|
Fix test, comments are now added when creating a table in PostgreSQL. |
|
Just to clarify the title of the PR... Postgres column comments are already supported. They just don't work. But it's cool that you have a PR to fix this, thank you! |
nionoku
changed the title
Indeed, but the moment I was thinking about the name, there was a slightly different thought in my head. Thanks for comment. |
|
Yeah, not trying to be pedantic - I just thought it was worth pointing out :) <3 |
|
Also, just to add - I've noticed that column comments actually already work (sort of) with postgres.....if you create a column it doesn't add the column comment in the migration. But if you somehow change something about that column (the length, the type etc.) the changes written into a migration also include the column comment. |
Yes, a comment for the column was added when changing the model fields. |
|
@rmainseas hello! Thank you for your attention to the question, regarding the attached issue #3357, I created a small repository, as typeorm I used my fork accordingly, and tested: created an entity, synchronized it with the database, conducted a test: the first module was executed, the second failed (as expected), then I changed the length of the field in the entity and repeated synchronization with the database, and repeated testing - all tests have been passed. That is, the comment is saved (by the way, this functionality is not included in my PR). |
|
This is really basic and simple, any reason it is not being merged? |
| // create column's comment | ||
| if (column.comment) { | ||
| upQueries.push(new Query(`COMMENT ON COLUMN ${this.escapePath(table)}.${column.name} IS '${column.comment}'`)); | ||
| downQueries.push(new Query(`COMMENT ON COLUMN ${this.escapePath(table)}.${column.name} IS '${column.comment}'`)); |
There was a problem hiding this comment.
column.name should be escaped here
|
|
||
| table.columns | ||
| .filter(it => it.comment) | ||
| .forEach(it => sql += ` COMMENT ON COLUMN ${this.escapePath(table)}.${it.name} IS '${it.comment}';`); |
There was a problem hiding this comment.
it.name should be escaped here
|
@nionoku we also need to obtain information about column comments from database and put it into ColumnMetadata for subsequent schema synchronization, like we did it with column charset and collation for example. e.g. in your current PR comment won't be applied on existing column. It also won't be removed once you remove comment from the decorator (or change existing comment) |
|
Any luck on addressing these issues? Without pulling it into column metadata, synchronize will stop working. Can you rebase this & fix conflicts, as well? |
|
Related issue #3360 |
imnotjames
left a comment
imnotjames
left a comment
There was a problem hiding this comment.
This needs to update the change detection code to handle checking for comment changes.
Also comments need to be escaped somehow
5 participants
Adding a comment to a column when creating a column for postgres.
Related #4359