#18468 closed New feature (fixed)
Add the ability to define comments in table / columns
Reported by: | Marc Rechté | Owned by: | KimSoungRyoul |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | niwi@…, Ivan Chernoff | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Database-level comments are valuable for database administrators, data analysts, data scientists, and others who are looking to consume data that is managed by Django. Most Django-supported databases also support table-level and column-level comments. This ticket would add functionality to Django to allow Django users to specify comments for syncdb manage.py to enter into the database.
....
....
new proposal (kimsoungryoul : 2020.03.23)
We will develop the code such as below
class AModel(models.Model): aaa = model.CharField(help_text="i am help_text", db_column_comment="i am db_comment",~~~) class Meta: db_table = "a_model_example_name" db_table_comment ="this is a_model comment ~~~~"
Change History (45)
comment:2 by , 12 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
This is a duplicate of #13867, which lingered in DDN for two years until I wontfix'd it.
The only argument I've seen is "it'd be nice if...". But I'm against adding features to Django just because we can; there must be a use case.
Could you start a thread on the mailing-list, as recommended in the contributing guide, to see if there is support for this idea?
If there's a good explanation of why you need this, I can reconsider my decision.
comment:3 by , 12 years ago
I understand your opinion! In any case, I'll write an email to the list which I think is useful.
Thanks!
comment:4 by , 12 years ago
I assume that most databases have comment on fields / tables feature not just because it is nice. Accessing Django created tables from another tool (for instance pgAdmin) would just make the system more productive. Personally I never create SQL tables without proper comments in the database itself.
comment:5 by , 12 years ago
The reasons given on #13867 still stand — Django is not aiming to provide a wrapper for every SQL feature. For example, it also doesn't provide an easy way to create stored procedures, functions or views, but you can always execute the SQL manually to add these, or could add some additional Python code that executed the SQL — for example using a South migration — if you want to ensure it always happens.
In addition, if the audience of these comments is people administering the database without reading the Python source code, it doesn't make sense for these comments to be taking up space in the Python code, which has its own way of adding comments (docstrings and comment lines), which are targeted at programmers not DB admins.
comment:6 by , 8 years ago
Resolution: | duplicate |
---|---|
Status: | closed → new |
Now that migration is built in Django, which can be used to create and administer the database, I again request this feature to be added. Thanks.
comment:7 by , 8 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Summary: | Define COMMENT in table / colomns → Add the ability to define comments in table / columns |
Type: | Uncategorized → New feature |
The correct way to reopen a ticket closed as "wontfix" is to start a discussion on the DevelopersMailingList. If there is consensus there to add the feature, then we reopen the ticket.
follow-up: 13 comment:8 by , 7 years ago
Cc: | added |
---|---|
Has patch: | unset |
Resolution: | duplicate |
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Version: | 1.4 → master |
After discussion on mailing list, the feature is good and can be added. (https://groups.google.com/forum/?nomobile=true#!topic/django-developers/guVTzO3RhUs)
I'll prepare patch for review.
follow-up: 10 comment:9 by , 7 years ago
I guess the model field option could be called db_column_comment
. I closed #28407 (introspecting column comments) as a duplicate since that should be implemented as part of this.
comment:10 by , 7 years ago
Replying to Tim Graham:
I guess the model field option could be called
db_column_comment
. I closed #28407 (introspecting column comments) as a duplicate since that should be implemented as part of this.
I think that we don't need new param, because comment
for django admin may be useful to store in database. But I can't decide on implementation, can you give me an advice?
Postgres and oracle have a syntax like comment on {table}.{column}
for storing comments, so this needs to be done after table/column creation, so there are two ways:
- Add it to post migrate signal, as for content types. But I can implement it as a third-party lib
- Add this SQL after database creation in schema.py
Which way is better?
comment:12 by , 7 years ago
I'm not sure what you mean by "comment for django admin". There isn't an existing option with that name.
As for the implementation, the second approach sounds better.
follow-up: 14 comment:13 by , 5 years ago
Replying to Ivan Chernoff:
After discussion on mailing list, the feature is good and can be added. (https://groups.google.com/forum/?nomobile=true#!topic/django-developers/guVTzO3RhUs)
I'll prepare patch for review.
Any news on this patch? It looks like there is a green light for one, and Tim already answered about the preferable approach. Do you need any help on making this hapen? It would be a fantastic feature for Django!
comment:14 by , 5 years ago
Replying to Rodrigo Silva:
Replying to Ivan Chernoff:
After discussion on mailing list, the feature is good and can be added. (https://groups.google.com/forum/?nomobile=true#!topic/django-developers/guVTzO3RhUs)
I'll prepare patch for review.
Any news on this patch? It looks like there is a green light for one, and Tim already answered about the preferable approach. Do you need any help on making this hapen? It would be a fantastic feature for Django!
I've a small library based on contenttype design: on post-migrate it copies all comments to database (PostgreSQL only).
https://github.com/vanadium23/django-db-comments
comment:15 by , 5 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Owner: | changed from | to
Patch needs improvement: | set |
Status: | new → assigned |
comment:16 by , 5 years ago
Description: | modified (diff) |
---|
i assign it to me
i will close this pullRequest caused by imperfection
https://github.com/django/django/pull/12605
I will bring a new PullRequest as soon as possible.
in at least a week
comment:17 by , 5 years ago
Description: | modified (diff) |
---|
comment:18 by , 5 years ago
Description: | modified (diff) |
---|
comment:19 by , 5 years ago
Description: | modified (diff) |
---|
comment:20 by , 5 years ago
Description: | modified (diff) |
---|
comment:21 by , 5 years ago
Description: | modified (diff) |
---|
comment:22 by , 5 years ago
Description: | modified (diff) |
---|
comment:23 by , 5 years ago
Adding a new setting is always a bit controversial. This should be discussed first on DevelopersMailingList. Personally I don't think that we need it.
comment:24 by , 5 years ago
i agree. your opinion.
i remove new settings. ENABLE_DB_COMMENT_WITH_HELP_TEXT
AND
i added few a more commit for support postgresql oracle
comment:25 by , 5 years ago
here is mail list for this feature
https://groups.google.com/forum/?nomobile=true#!topic/django-developers/guVTzO3RhUs
comment:26 by , 5 years ago
Description: | modified (diff) |
---|
comment:27 by , 5 years ago
If you don't mind me asking...
could you give me a feedback to this feature?
comment:28 by , 5 years ago
Hi felixxm
I want this feature to be reflected in Django.3.1
I know this feature that many people won't use.
but some people who communicate with DBA would be helpful this feature
sometimes ORM take control from DBA
like these situation...
DBAs: we want to know comment(help_text) not to *.py could you use COMMENT SQL in migrationFile? Developers: No, It's Impossible because django ORM does not support COMMENT SQL DBAs: ???? that means .. should we refer *.py file everytimes even though COMMENT SQL exists but can't use? DBAs: it's illogical!!!! Developers: but if we use COMMENT SQL, we customize everytimes to generate migrate files Developers: it's inefficient!!!!
That's one of the reasons for avoiding ORM.
If you don't mind me asking...
could you give me a feedback to this feature?
comment:30 by , 4 years ago
Rebased a previous PR to add support to inspectdb here: https://github.com/django/django/pull/13737
Still missing some tests and it looks like we would like it to be a verbose_name field and not a comment
comment:31 by , 3 years ago
Owner: | changed from | to
---|
Owner updated based on suggestion in django-developers group https://groups.google.com/g/django-developers/c/guVTzO3RhUs
comment:32 by , 3 years ago
Status update: KimSoungRyoul has created a new PR (here) that addresses some of the suggestions raised by atombrella in May 2020 on the previous PR (here). Huge thanks to KimSoungRyoul for resuming work on this feature!
The new PR is not complete yet. Still needed:
1) Test coverage.
2) Additional changes to the proposed docs including further explanation, a warning about lack of SQLite support, and a warning about overwriting comments made by DBAs in the db, versionadded annotation, and a couple other things.
3) Release note.
4) General code review, especially related to where sqlite warnings should be triggered in the code to avoid having hard-coded vendor names in django/db/models/base.py.
Next steps: I'll add some suggestions directly to the new PR and post updates here as this PR progresses.
comment:33 by , 3 years ago
Update: This feature is still being actively developed. KimSoungRyoul has been leading the development, and we've received input as well from knyghty on the PR. Docs and unit tests have been written, and right now we are discussing edge cases that need to be properly handled or explicitly "unsupported" (such as, for example, Django model classes or fields that Django doesn't actually create in the db such as M2M fields, proxy models, abstract base classes, and more).
comment:34 by , 3 years ago
Description: | modified (diff) |
---|---|
Needs documentation: | unset |
Needs tests: | unset |
Patch needs improvement: | unset |
This patch is ready for review. It meets everything on the checklist. I'm updating the status so it shows up in Patches Needing Review
on the Development Dashboard.
If you would like additional information about some of the design and implementation decisions behind this patch, please refer to the Github Pull Request conversation at https://github.com/django/django/pull/14463
comment:35 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Update: This patch has been reviewed (refer to this comment on the Github PR for the details on the patch review checklist for this patch). I have marked it "Ready for checkin"
Note: Although I am the owner of this ticket, I am NOT the author of this patch. The author of the patch is KimSoungRyoul. Because I am not the author, I conducted the patch review using the patch review checklist.
comment:37 by , 3 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:38 by , 3 years ago
Status update: KSR has updated the patch on github https://github.com/django/django/pull/14463 and there's continued discussion there, including additional changes proposed by other community members...
comment:39 by , 2 years ago
Hi a few days ago I fix pullrequest to target release 4.2
Fixed an issue on the Oracle side
comment:40 by , 2 years ago
Patch needs improvement: | unset |
---|
After updating a patch, you can uncheck "Patch needs improvement" on the ticket so that it appears in the review queue.
comment:41 by , 2 years ago
Patch needs improvement: | set |
---|
comment:42 by , 23 months ago
Owner: | changed from | to
---|
comment:43 by , 23 months ago
Patch needs improvement: | unset |
---|
comment:44 by , 23 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Initial implementation for postgresql_psycopg2 backend: https://github.com/niwibe/django/compare/issue_18468
Is only implemented the table comment sql.
If accepted as a new feature, would be nice to make design decisions. Today I put the comment for table in class "Meta", another option is to use the first line of the model docstrings.
For fields, is more simple, just add an optional parameter "comment" at the Field.