Missing SQL constraint to proxy models

[thibaultj]

In a model class, when you add a ForeignKey to a proxy model, the sql reference will no be created in Mysql. This is a severe issue, since the integrity structure of the database cannot be ensured.

Moreover, as stated in #16128, the django admin will not gather related proxy model objects when deleting something. This is a separate issue though, since we are talking of the sql generation here.

# file constraint_test/models.py
class Parent(models.Model):
    name = models.CharField(max_length=25)

class Father(Parent):
    class Meta:
        proxy = True

    def __unicode__(self):
        return "I'm your father, luke"

class Child(models.Model):
    name = models.CharField(max_length=25)
    parent = models.ForeignKey(Parent, related_name='child_parent_set')
    father = models.ForeignKey(Father, related_name='child_father_set')

python manage.py sql constaint_test

BEGIN;
CREATE TABLE `constraint_test_parent` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
    `name` varchar(25) NOT NULL
)
;
CREATE TABLE `constraint_test_child` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
    `name` varchar(25) NOT NULL,
    `parent_id` integer NOT NULL,
    `father_id` integer NOT NULL
)
;
ALTER TABLE `constraint_test_child` ADD CONSTRAINT `parent_id_refs_id_875b6ff2` FOREIGN KEY (`parent_id`) REFERENCES `constraint_test_parent` (`id`);
-- Missing constraint to the father table
COMMIT;

[Anssi Kääriäinen]

Sorry for the stupid question, but what is the point of allowing foreign keys to proxy models at all? In the DB the relation is to the base class anyways. The only point I can see is you want to return instances of the proxy class instead of the base class, but I hope that could be fixed by something like Parent.objects.as_model(Father). This would have uses beyond just ForeignKeys.

My point is that is it worth fixing this by complicating the ForeignKey implementation? You don't get that much from the work after all. I haven't actually checked what it would need to fix this. One possibility is to just make the ForeignKey in reality to the base class and when constructing the related query set, just start from the Father model.

If it is easy to fix the current implementation, by all means lets do that. But I am not too sure that is the case here...

[thibaultj]

I don't think this is directly related to the ForeignKey implementation. No matter how the python model and class inheritance is handled, data integrity must be respected on the db level.

Consider the following exemple:

random_id = 666
child = Child()
child.name = 'random name
child.parent_id = random_id
child.save() # Throws IntegrityError
child.father_id = random_id
child.save() # No exception thrown

That means that you can have crappy data in db, which breaks the admin modules for those models.

[jenh]

I am getting the same behaviour with SQLite and PostgreSQL, only with a slight twist. If the ForeignKey is not given a related_name, the SQL will include a reference to the parent model. It is only when a related_name is present that the SQL will not include a reference. This strikes me as rather odd, but I haven't ever delved into the database code.

As for the use of specifying the proxy model in the model definition, I see it as a matter of clarity. The ORM provides useful abstractions, and this is one - the reference really is, in the example, to a Father and not to a Parent, and the fact that this distinction does not exist at the database level is immaterial at the ORM level.

[jenh]

After further investigation, it appears to be merely coincidence that in my cases the presence of related_name corresponds exactly with whether the constraint is created. Sorry for that red herring.

The reason some of my ForeignKeys get constraints is that they are handled in db/backends/creation.py BaseDatabaseCreation.sql_for_inline_foreign_key_references, when the proxy model being referenced happens to be in the list of the app's models that have been processed at the point it reaches this non-proxy model. The SQL generator has no problem with the fact that the referenced model is a proxy model, since it uses field.rel.to._meta.db_table, which matches the parent model.

The reason the other ForeignKeys don't get constraints is that if the model containing the references is processed before the proxy models it is referencing, the references get deferred and are handled in BaseDatabaseCreation.sql_for_pending_references. Crucially, this method operates on the model being referenced, and thus referenced proxy models fall afoul of its ditching of unmanaged and proxy models.

Now, it seems to me that there is no reason for sql_for_pending_references to not do anything with proxy models. The constraints it creates use the underlying database table and are perfectly fine, and if there are no pending references to the proxy model, the method does nothing. However, being completely new to this part of Django's code, I may well be missing something important.

I'm attaching a patch that just removes the requirement that a model not be a proxy model in order to create pending references to it. I don't have a test that this fixes the problem; if someone can give me some pointers on where that test should go and how it should be formulated, I'll do my best to create one. It does at least not cause any of the existing tests to fail (checked under PostgreSQL and SQLite (which doesn't use the code that's changed)).

[jenh]

Patch to remove exclusion of proxy models from creation of pending references.

[Simon Charette]

Added tests

[Simon Charette]

Added the missing tests. They both pass on sqlite (tests are skipped because the backend doesn't enforce constraints) and postgresql.

[jenh]

Is there anything else that I can do to progress this, or are we just needing a reviewer to mark as ready for checkin (or state why it isn't)? I'm not sure what if any documentation should be added.

[Tim Graham]

Verified that the tests fail on MySQL before the fix and pass on all backends after (didn't try Oracle). I'm not confident enough to check this in myself, but am marking it as such so someone else can verify. Here's a ​PR that merges cleanly.

[Tim Graham <timograham@…>]

In 196cc875b26f266e8f8dfd5be9daa0b8f246b9cd:

[1.6.x] Fixed #17519 -- Fixed missing SQL constraints to proxy models.

Thanks thibaultj for the report, jenh for the patch,
and charettes for the tests.

Backport of aa830009de from master

[Tim Graham]

In aa830009de940e31711b4858501becd7f34a1972:

Fixed #17519 -- Fixed missing SQL constraints to proxy models.

Thanks thibaultj for the report, jenh for the patch,
and charettes for the tests.