[Django] #20098: Django validate command fails to detect that multiple models declare the same db

Django

unread,

Mar 20, 2013, 11:10:48 AM3/20/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table
----------------------------------------------+--------------------
Reporter: carsten.klein@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
When declaring two models using a custom db_table setting in the Meta
class, I found out that django failed to detect that the two models
declared the same db_table and subsequently it will fail on syncdb when
trying to create the same table twice.

This, of course, was introduced by copy&paste, but at least django should
report this on validate instead of failing when trying to syncdb.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Mar 20, 2013, 12:34:08 PM3/20/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):

* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* type: Bug => Cleanup/optimization
* needs_tests: => 0
* needs_docs: => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:1>

Django

unread,

Mar 20, 2013, 2:40:51 PM3/20/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):

* cc: bmispelon@… (added)
* has_patch: 0 => 1
* needs_tests: 0 => 1

Comment:

This seems like a good idea but it needs to have tests in order for it to
be merged.

From what I can tell, the tests for the `validate` command go in
`tests/admin_scripts/tests.py`:
https://github.com/django/django/blob/master/tests/admin_scripts/tests.py#L1046

Also, consider adding a `.diff` extension to your patch so that trac can
use syntax highlighting on it.

Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:2>

Django

unread,

Mar 20, 2013, 4:42:06 PM3/20/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by Keryn Knight <django@…>):

It is worth bearing in mind that there are valid use cases for
encountering duplicate table names, such as when using proxy models, or
unmanaged models. The provided patch doesn't seem to account for that, but
I'm not familiar enough with the surrounding code to know whether it is
already handled.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:3>

Django

unread,

Mar 22, 2013, 9:42:58 AM3/22/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by carsten.klein@…):

Thanks for pointing this out. I included a revised version of the patch
including also two test cases, one that tests against a single application
declaring duplicate db tables and one that tests against multiple
applications declaring duplicate db tables.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:4>

Django

unread,

Mar 22, 2013, 9:46:58 AM3/22/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by anonymous):

As proxies reuse the actual model the newly provided patch should be
sufficient.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:5>

Django

unread,

Mar 22, 2013, 9:48:38 AM3/22/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by carsten.klein@…):

Replying to [comment:4 carsten.klein@…]:

> Thanks for pointing this out. I included a revised version of the patch
including also two test cases, one that tests against a single application
declaring duplicate db tables and one that tests against multiple
applications declaring duplicate db tables.

As far as unmanaged models go, these should not be using the ModelBase
meta class then, or would they?

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:6>

Django

unread,

Mar 22, 2013, 9:52:30 AM3/22/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by bmispelon):

There's a typo in the added comment:
> Lookup table for table names introduced in order to prevent that users
from declaring the same table twice

Regarding unmanaged and proxy models, I'm not familiar enough with the
code to tell if they are handled with this patch but one surefire way to
tell would be to add some tests that use them.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:7>

Django

unread,

Mar 22, 2013, 9:56:03 AM3/22/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by carsten.klein@…):

Included the newly added apps for backing the test cases.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:8>

Django

unread,

Mar 22, 2013, 10:00:07 AM3/22/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by carsten.klein@…):

As a side effect this should also detect copy&paste errors such as
duplicating a model class and failing to rename it and updating its Meta
accordingly.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:9>

Django

unread,

Mar 25, 2013, 5:12:03 AM3/25/13

to django-...@googlegroups.com

#20098: Django validate command fails to detect that multiple models declare the
same db_table

-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: nobody

Comment (by carsten.klein@…):

Replying to [comment:3 Keryn Knight <django@…>]:

> It is worth bearing in mind that there are valid use cases for
encountering duplicate table names, such as when using proxy models, or
unmanaged models. The provided patch doesn't seem to account for that, but
I'm not familiar enough with the surrounding code to know whether it is
already handled.

Just had a look at the documentation on unmanaged models. It seems that
this is a special case that needs to be dealt with. I will add a guard to
the patch and a test case that also includes unmanaged models.

Thanks for pointing this out!

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:10>

Django

unread,

Aug 4, 2015, 7:39:53 PM8/4/15

to django-...@googlegroups.com

#20098: Add validation for models with the same db_table
--------------------------------------+------------------------------------
Reporter: carsten.klein@… | Owner: nobody
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by timgraham):

* type: Cleanup/optimization => New feature
* version: 1.5 => master
* component: Database layer (models, ORM) => Core (System checks)

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:11>

Django

unread,

Oct 26, 2018, 9:05:19 PM10/26/18

to django-...@googlegroups.com

#20098: Add validation for models with the same db_table

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sanyam Khurana):

* owner: nobody => Sanyam Khurana
* status: new => assigned

Comment:

There is a lot of regression in applying the patch cleanly which revolve
mostly around the test cases within Django. I'm working to modify the last
version of the patch, cleanly apply it to the master branch and fix test
cases.

PR will be up soon.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:12>

Django

unread,

Dec 3, 2018, 6:04:06 AM12/3/18

to django-...@googlegroups.com

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin

Comment:

Not sure if it's worth adding a co-author given the final patch is quite
different from the original one here.

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:13>

Django

unread,

Dec 22, 2018, 11:44:52 AM12/22/18

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by Sanyam Khurana):

Hello Tim,

I've addressed your reviews on the patch. Can you please have a look at
this when you've some time?

Please let me know if there are any more changes needed.

Thanks!

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:14>

Django

unread,

Dec 24, 2018, 11:15:41 AM12/24/18

to django-...@googlegroups.com

#20098: Add validation for models with the same db_table
-------------------------------------+-------------------------------------
Reporter: carsten.klein@… | Owner: Sanyam
| Khurana

Type: New feature | Status: closed

Component: Core (System | Version: master
checks) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"5d25804eaf81795c7d457e5a2a9f0b9b0989136c" 5d25804]:
{{{
#!CommitTicketReference repository=""
revision="5d25804eaf81795c7d457e5a2a9f0b9b0989136c"
Fixed #20098 -- Added a check for model Meta.db_table collisions.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/20098#comment:15>

Django

unread,

Aug 2, 2019, 12:01:20 PM8/2/19

to django-...@googlegroups.com

[Django] #20098: Django validate command fails to detect that multiple models declare the same db_table

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django