Opened 8 years ago

Last modified 6 weeks ago

#25902 new New feature

Add system check for project-wide database table name conflicts

Reported by: Amos Onn Owned by: nobody
Component: Core (System checks) Version: dev
Severity: Normal Keywords:
Cc: Ülgen Sarıkavak Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Amos Onn)

If you have an app called X with a model called Y with a m2m field called Z, the default table name for the m2m is X_Y_Z.
If you have an app called X_Y with a model called Z, the default table name for the model is also X_Y_Z.
If you have both, django breaks when creating the tables.
See a broken sample project at:
https://github.com/amosonn/check_underscore

Change History (5)

comment:1 by Amos Onn, 8 years ago

Description: modified (diff)

comment:2 by Tim Graham, 8 years ago

Component: Database layer (models, ORM)Core (System checks)
Summary: app names with underscore conflict with many-to-many tablesAdd system check for project-wide database table name conflicts
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature
Version: 1.9master

I guess we might be able to add a system check for these sort of clashes, but it might be a non-trivial amount of work for what I assume isn't a very common case. Also, such validation would need to account for the possibility that a user does want two models to use the same database table. Probably Meta.db_table would be set by the user in such cases. Do you have any other ideas? #12810 is another ticket about checks for clashing many-to-many tables.

comment:3 by Amos Onn, 8 years ago

I also noticed the other bug, but thought the behaviour django exhibits there is more pleasant: it raises a clean, pythonic, internal error, and does so when generating the migrations. In this case, it generates the migrations silently, and then fails with a db error when applying them.

I'm unsure, but I think that a user who wants two models for the same table, currently won't be able to do so without manually editing the migrations, running one of the CreateModel-s (but not the other) with SeparateDatabaseAndState - otherwise the second one will attempt creating the table a second time, and the same db error will be raised.

So, what I think is that perhaps the system check is something that should be run at makemigrations time, and issue a warning such as: "The automatically generated migrations have two models ($names) with the same database table, and will break when run. Consider giving those models an explicit db_table. If you are modifying the migrations manually, you can ignore this error." This covers both the case I described, in which the user will go and give one of them (presumably the m2m) a name, and the case you described, in which the user knows that he's doing something strange, and will get a reminder that this has to be reflected in migrations. It also covers a third case, in which the exact same app name appears as two different submodules in the project, and which might actually be more probable than both our previous cases.

I think such a check should not be too costy; perhaps I can add it, maybe with some guidance as to where is the optimal place for it to reside.

I finally add that my original thought about this was that the problem was with the default django table names. I still think that a table name of shoppingmall_parking is better than shopping_mall_parking for the model shopping_mall.Parking - just as in model names, the separation between words using camelcasing is removed, so should be the separation between words using underscores in app names. It is true that this doesn't cover all cases, for instance some one with both x_y and xy as app names, but that is even more uncommon.

comment:4 by Tim Graham, 8 years ago

I think the argument for having this check outside of the migrations framework is that if you are combining existing third-party apps, you won't necessarily be generating migrations for them. The check might go in django/django/db/models/base.py (see the check() method). There would need to be a global registry of tables names.

We likely can't change the default table names that Django generates for backwards compatibility reasons.

comment:5 by Ülgen Sarıkavak, 6 weeks ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top