Opened 16 months ago

Last modified 14 months ago

#30355 new Bug

Specifying custom manager doesn't work with prefetch

Reported by: Kyle Mulka Owned by:
Component: Documentation Version: master
Severity: Normal Keywords:
Cc: Kourt Bailey Triage Stage: Accepted
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using prefetch and specifying a custom manager to use for a reverse relation, Django doesn't filter correctly. Here's an example:

class Business(models.Model):
    name = models.CharField(max_length=255)

    def approved_reviews(self):
        return self.review_set(manager='approved_reviews').all()


class ApprovedReviewsManager(models.Manager):
    def get_queryset(self):
        return super().get_queryset().filter(status=Review.APPROVED)


class Review(models.Model):
    NEW = 1
    APPROVED = 2
    STATUS_CHOICES = (
        (NEW, 'New'),
        (APPROVED, 'Approved'),
    )
    business = models.ForeignKey(Business)
    text = models.CharField(max_length=255)
    status = models.IntegerField(choices=STATUS_CHOICES, default=NEW)

    objects = models.Manager()
    approved_reviews = ApprovedReviewsManager()


class ApprovedReviewsTest(TestCase):
    def test_with_prefetch(self):
        business = Business()
        business.save()

        review = Review()
        review.business = business
        review.save()

        businesses = Business.objects.prefetch_related('review_set').all()

        business = businesses[0]
        approved_reviews = business.review_set(manager='approved_reviews').all()

        self.assertEqual(len(approved_reviews), 0)

The full test project is available here: https://github.com/mulka/django_prefetch_manager_bug/blob/master/review_site/tests.py

Change History (7)

comment:1 Changed 16 months ago by Daniel Hepper

I've reproduced this behavior with the master branch (the original report was for 1.11), but I don't know if this is actually a bug or just a case of a somewhat unintuitive API. This should at least be documented in the section on using a custom reverse manager: https://docs.djangoproject.com/en/2.2/topics/db/queries/#using-a-custom-reverse-manager

A possible workaround would be to use a django.db.models.Prefetch object:

class ApprovedReviewsTest(TestCase):
    def test_with_prefetch(self):
        business = Business()
        business.save()

        review = Review()
        review.business = business
        review.save()

        prefetch_approved_reviews = Prefetch('review_set', queryset=Review.approved_reviews.all())
        businesses = Business.objects.prefetch_related(prefetch_approved_reviews).all()

        business = businesses[0]
        approved_reviews = business.review_set(manager='approved_reviews').all()

        self.assertEqual(len(approved_reviews), 0)

In this example, you don't even have to specify the custom manager, business.review_set.all() would give you the same result.

The underlying issue is that there is only a single prefetched object cache per field, see https://github.com/django/django/blob/de7f6b51b21747e19e90d9e3e04e0cdbf84e8a75/django/db/models/fields/related_descriptors.py#L607

Last edited 16 months ago by Daniel Hepper (previous) (diff)

comment:2 Changed 16 months ago by Daniel Hepper

Triage Stage: UnreviewedAccepted

I was about to write a patch for the docs on "Using a custom reverse manager" to highlight the implication of using prefetch with custom managers.

But thinking about it further made me realize that the prefetch might have happened in a completely unrelated part of the code. As a conclusion, I'd consider this a bug.

comment:3 Changed 16 months ago by Daniel Hepper

Version: 1.11master

comment:4 Changed 15 months ago by Kourt Bailey

Cc: Kourt Bailey added

comment:5 Changed 15 months ago by robinh00d

Owner: changed from nobody to robinh00d
Status: newassigned

comment:6 Changed 14 months ago by robinh00d

Owner: robinh00d deleted
Status: assignednew

comment:7 Changed 14 months ago by Simon Charette

Component: Database layer (models, ORM)Documentation
Needs documentation: set

I agree that it's more of a documentation issue as prefetch_related will use the default manager just like accessing a related manager does.

In other words Business.objects.prefetch_related('review_set') will use the default manager just like business.review_set.all() does.

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