Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#19816 closed Bug (fixed)

Assigning a QS to a M2M relation that was generated from that M2M relation clears both the QS and the relation

Reported by: jedediah Owned by: Aleksandra Sendecka
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: sprint2013
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

from django.db import models

class Topping(models.Model):
    meat = models.BooleanField()

class Pizza(models.Model):
    toppings = models.ManyToManyField(Topping)
>>> pizza = Pizza.objects.create()
>>> cheese = Topping.objects.create(meat=False)
>>> bacon = Topping.objects.create(meat=True)
>>> pizza.toppings = [cheese, bacon]
>>> pizza.toppings.count()
2
>>> vegs = pizza.toppings.filter(meat=False)
>>> vegs.count()
1
>>> pizza.toppings = vegs
>>> pizza.toppings.count()
0
>>> vegs.count()
0

Would expect vegs to be unchanged, and pizza.toppings to be [cheese]

The problem is the same if the vegs QS is generated with Topping.objects.filter(pizza=pizza). If it comes from somewhere not involving the relation then everything works as expected.

Attachments (1)

ticket19816.diff (1.8 KB ) - added by Aleksandra Sendecka 11 years ago.
Patch with tests added. Done by me and oinopion.

Download all attachments as: .zip

Change History (12)

comment:1 by Carl Meyer, 11 years ago

Triage Stage: UnreviewedAccepted

Yes, because the queryset isn't evaluated until after the existing m2m values are cleared, meaning it evaluates to an empty queryset. Definitely a bug. Thanks for the report!

comment:2 by Aleksandra Sendecka, 11 years ago

Owner: changed from nobody to Aleksandra Sendecka
Status: newassigned

by Aleksandra Sendecka, 11 years ago

Attachment: ticket19816.diff added

Patch with tests added. Done by me and oinopion.

comment:3 by anonymous, 11 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:4 by anonymous, 11 years ago

Keywords: sprint2013 added

comment:5 by Tomek Paczkowski <tomek@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 3dddbc0f2380d2654e66c8df4bb40d5e1d42af98:

Fixed #19816: pre-evaluate queryset on m2m set

In ReverseManyRelatedObjectsDescriptor.set, evaluate possible
queryset to avoid problems when clear() would touch data this queryset
returns.

comment:6 by Honza Král <Honza.Kral@…>, 11 years ago

In 6cb6e1128f090d5edde9918ba2ae79c1e38b65ad:

Merge pull request #769 from oinopion/ticket19816

Fixed #19816 -- pre-evaluate queryset on m2m set

comment:7 by loic84, 10 years ago

This PR fixes the issue for the other types of descriptors: https://github.com/django/django/pull/2487.

comment:8 by Loic Bistuer <loic.bistuer@…>, 10 years ago

In 20399083f49faceef1e48530822137257bca9d6a:

Fixed #19816 -- Pre-evaluate querysets used in direct relation assignments.

Since assignments on M2M or reverse FK descriptors is composed of a clear(),
followed by an add(), clear() could potentially affect the value of the
assigned queryset before the add() step; pre-evaluating it solves the problem.

This patch fixes the issue for ForeignRelatedObjectsDescriptor,
ManyRelatedObjectsDescriptor, and ReverseGenericRelatedObjectsDescriptor.
It completes 6cb6e1 which addressed ReverseManyRelatedObjectsDescriptor.

comment:9 by Aymeric Augustin <aymeric.augustin@…>, 10 years ago

In a2407c9577c400bf9931ff1db1d7757afa378162:

Merge pull request #2487 from loic/ticket19816

Fixed #19816 -- Pre-evaluate querysets used in direct relation assignments.

comment:10 by Loic Bistuer <loic.bistuer@…>, 10 years ago

In da7ab8728c795e1853cea4215469ae3649664c6d:

Fixed mistake in tests from commit 2039908. Refs #19816.

comment:11 by Aymeric Augustin <aymeric.augustin@…>, 10 years ago

In 9723e999a0b4acb212210241851dbe75dfbc1a55:

Merge pull request #2495 from loic/ticket19816

Fixed mistake in tests from commit 2039908. Refs #19816.

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