Opened 3 years ago

Closed 3 years ago

Last modified 17 months 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: ethlinn
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 ethlinn 3 years ago.
Patch with tests added. Done by me and oinopion.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 3 years ago by carljm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 3 years ago by ethlinn

  • Owner changed from nobody to ethlinn
  • Status changed from new to assigned

Changed 3 years ago by ethlinn

Patch with tests added. Done by me and oinopion.

comment:3 Changed 3 years ago by anonymous

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

comment:4 Changed 3 years ago by anonymous

  • Keywords sprint2013 added

comment:5 Changed 3 years ago by Tomek Paczkowski <tomek@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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 Changed 3 years ago by Honza Král <Honza.Kral@…>

In 6cb6e1128f090d5edde9918ba2ae79c1e38b65ad:

Merge pull request #769 from oinopion/ticket19816

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

comment:7 Changed 17 months ago by loic84

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

comment:8 Changed 17 months ago by Loic Bistuer <loic.bistuer@…>

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 Changed 17 months ago by Aymeric Augustin <aymeric.augustin@…>

In a2407c9577c400bf9931ff1db1d7757afa378162:

Merge pull request #2487 from loic/ticket19816

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

comment:10 Changed 17 months ago by Loic Bistuer <loic.bistuer@…>

In da7ab8728c795e1853cea4215469ae3649664c6d:

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

comment:11 Changed 17 months ago by Aymeric Augustin <aymeric.augustin@…>

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