4

while writing a testcase for a method I discovered that I get different results if I use my_queryset.first().my_annotated_value than when I use my_queryset.last().my_annotated_value although my_queryset.count() returns 1.

Here is the relevant code snippet:

class ShopManager(models.Manager):

def get_best_matches(self, customer):

    shops = super(ShopManager, self).get_queryset().filter(employees__matches__customer=customer).annotate(max_match_percentage=Coalesce(Max('employees__matches__match_value'), 0)).order_by('-max_match_percentage')

    for shop in shops:
        shop.max_match_percentage = float(shop.max_match_percentage) * 100.0

    return shops

In the shell I run:

shops = Shop.objects.get_best_matches(customer=Customer_A)
shops.count() # returns 1

shops.first().max_match_percentage # returns 73.9843
shops.last().max_match_percentage # returns Decimal('0.739843')

I have different django apps for shops, matches, employees and customers.

I searched for hours and checked the implementation for first() and last() in the django docs. I could not find anything that explains this behavior.

Why are the values different, what exactly is happening? Am I doing something wrong or is this a bug?

jimfawkes
  • 357
  • 2
  • 12
  • 1
    Can you try running the `shops.first().max_match_percentage` and `shops.last().max_match_percentage` in reverse order. What are the results – tushortz Jun 02 '17 at 15:33
  • @Tushortz I just tried that, the result is the same. `shop.last().max_match_percentage` still returns `Decimal('0.739843')` and `shop.first().max_match_percentage` still returns `73.9843` – jimfawkes Jun 02 '17 at 15:58
  • 1
    You have a data consistency problem. The queryset is cloned again when calling `last` which does not run the `for` loop in the manager method, so what you're seeing is the current value in the DB. See Alasdair's answer. – Moses Koledoye Jun 02 '17 at 16:08
  • Try to never evaluate the queryset in the manager. Managers are supposed to retrieve data from database, not alter them. – Ozgur Vatansever Jun 02 '17 at 16:21

1 Answers1

5

In your get_best_matches method, you evaluate the queryset when you loop through it, and modify shop.max_match_percentage for each item in the queryset.

When you call first(), Django returns the first item in the evaluated queryset.

When you call last(), Django attempts to return the first item of the reversed queryset. This is a new queryset and causes a new database lookup. You have not set shop.max_match_percentage for this queryset, so you get the decimal from the database.

As you can see, it's probably not a good idea to loop through a queryset and modify it before returning it from a model manager method. If the queryset is cloned (e.g. by a further filter(), order_by() or in this case last()), then the changes are lost.

You should be able to do the multiplication by 100 inside the queryset, instead of looping through it:

shops = super(ShopManager, self).get_queryset().filter(employees__matches__customer=customer).annotate(max_match_percentage=Coalesce(Max('employees__matches__match_value'), 0)*100).order_by('-max_match_percentage')

If you really need to return a float instead of a decimal field, you can use Cast.

Alasdair
  • 298,606
  • 55
  • 578
  • 516