Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#11956 closed (fixed)

ReverseManyRelatedObjectsDescriptor wrongly assumes symmetry between inherited objects

Reported by: Nicolas Dietrich Owned by: malcalypse
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Symmetrical many-to-many do only make sense between objects of the same model type, as described in the documentation. However, ReverseManyRelatedObjectDescriptors wrongly assume symmetricity when defining a ManyToManyField from an inherited model towards a base class, as in the following model definition:

class BaseModel(models.Model):
    pass

class ChildModel(BaseModel):
    many = models.ManyToManyField(BaseModel, related_name='many_children')

This results in the following integrity error in case Postgres is used (- for some reason, the problem does not occur using sqlite!):

> from symmetricbug.models import *
> base = BaseModel.objects.create()
> child = ChildModel.objects.create()
> child.many.add(base)

[...]

/usr/lib64/python2.5/site-packages/django/db/models/fields/related.pyc in add(self, *objs)
    432                 # If this is a symmetrical m2m relation to self, add the mirror entry in the m2m table
    433                 if self.symmetrical:
--> 434                     self._add_items(self.target_col_name, self.source_col_name, *objs)
    435             add.alters_data = True

[...]

IntegrityError: insert or update on table "symmetricbug_childmodel_many" violates foreign key constraint "symmetricbug_childmodel_many_childmodel_id_fkey"
DETAIL:  Key (childmodel_id)=(1) is not present in table "symmetricbug_childmodel".

This obviously does not make any sense, because there is no ChildModel object with id 1. The reason is that ReverseManyRelatedObjectsDescriptor wrongly assumes symmetricity:

django.db.models.fields.related:609 is

symmetrical=(self.field.rel.symmetrical and isinstance(instance, rel_model)),

and should be something alike

symmetrical=(self.field.rel.symmetrical and instance.__class__ == rel_model.__class__),

to be aware of subclasses.

Workaround: Use symmetrical=False in the model definition:

class ChildModel(BaseModel):
    many = models.ManyToManyField(BaseModel, related_name='many_children', symmetrical=False)

Change History (9)

comment:1 Changed 7 years ago by Alex Gaynor

It should be noted that the code used to read exactly what you suggested, however that prevented you from adding subclasses of the ManyToMany model to the relationship.

comment:2 Changed 7 years ago by Andrew Stoneman

I just ran into this, and it seems odd to me that the default value for symmetrical is decided on a per-instance basis, when the related manager is created, rather than just once, when the field is created. Would it make more sense to change django/db/models/fields/related:804 (in ManyToManyField.__init__) from:

            symmetrical=kwargs.pop('symmetrical', True),

to something like:

            symmetrical=kwargs.pop('symmetrical', to == RECURSIVE_RELATIONSHIP_CONSTANT),

and then assume it's set correctly farther in, and just remove the isinstance check on line 609?

comment:3 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:4 Changed 7 years ago by flyingfred0

Owner: changed from nobody to flyingfred0
Status: newassigned

comment:5 Changed 7 years ago by flyingfred0

Owner: changed from flyingfred0 to nobody
Status: assignednew

I had no luck reproducing this one with either psycopg or psycopg2 (against PostgreSQL 8.4.0) on OS X 10.5.

comment:6 Changed 7 years ago by malcalypse

Owner: changed from nobody to malcalypse
Status: newassigned

comment:7 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: assignedclosed

(In [12908]) Fixed #11956 -- Modified the handling of m2m relationships between subclasses. Thanks to nidi for the report, and astoneman for the suggestion on how to fix the problem.

comment:8 Changed 7 years ago by Russell Keith-Magee

(In [12909]) [1.1.X] Fixed #11956 -- Modified the handling of m2m relationships between subclasses. Thanks to nidi for the report, and astoneman for the suggestion on how to fix the problem.

Backport of r12908 from trunk.

comment:9 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top