[Django] #25902: app names with underscore conflict with many-to-many tables
Django
----------------------------------------------+--------------------
Reporter: amosonn | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
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:
--
Ticket URL: <https://code.djangoproject.com/ticket/25902>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: amosonn | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by amosonn):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> 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:
New description:
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
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25902#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: amosonn | Owner: nobody
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => New feature
* component: Database layer (models, ORM) => Core (System checks)
* version: 1.9 => master
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25902#comment:2>
Django
--------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by amosonn):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25902#comment:3>
Django
--------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25902#comment:4>
Django
--------------------------------------+------------------------------------
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Changes (by Ülgen Sarıkavak):
* cc: Ülgen Sarıkavak (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25902#comment:5>