Add validation for models with the same db_table

[carsten.klein@…]

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.

[carsten.klein@…]

Proposed patch to django.db.models.options.py that might resolve the issue (tested)

[Baptiste Mispelon]

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.

[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.

[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.

[anonymous]

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

[carsten.klein@…]

Replying to 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?

[Baptiste Mispelon]

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.

[carsten.klein@…]

Included the newly added apps for backing the test cases.

[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.

[carsten.klein@…]

Replying to 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!

[carsten.klein@…]

Correct handling of unmanaged models and duplicate db_table declaration

[carsten.klein@…]

Adds test case for unmanaged model classes

[Sanyam Khurana]

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.

[Simon Charette]

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

[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!

[Tim Graham <timograham@…>]

In 5d25804:

Fixed #20098 -- Added a check for model Meta.db_table collisions.

[Claude Paroz]

Note that a regression was spotted in #30673.