37

I have a super simple django model here:

class Notification(models.Model):
    message = models.TextField()
    user = models.ForeignKey(User)
    timestamp = models.DateTimeField(default=datetime.datetime.now)

Using ajax, I check for new messages every minute. I only show the five most recent notifications to the user at any time. What I'm trying to avoid, is the following scenario.

User logs in and has no notifications. While the user's window is up, he receives 10 new messages. Since I'm only showing him five, no big deal. The problem happens when the user starts to delete his notifications. If he deletes the five that are displayed, the five older ones will be displayed on the next ajax call or refresh.

I'd like to have my model's save method delete everything but the 5 most recent objects whenever a new one is saved. Unfortunately, you can't use [5:] to do this. Help?

EDIT

I tried this which didn't work as expected (in the model's save method):

    notes = Notification.objects.filter(user=self.user)[:4]
    Notification.objects.exclude(pk__in=notes).delete()

i couldn't find a pattern in strange behavior, but after a while of testing, it would only delete the most recent one when a new one was created. i have NO idea why this would be. the ordering is taken care of in the model's Meta class (by timestamp descending). thanks for the help, but my way seems to be the only one that works consistently.

Brandon Henry
  • 3,632
  • 2
  • 25
  • 30
  • Useful to underline that the fact that you can't use delete on a slice is explained in the doc: https://docs.djangoproject.com/en/dev/ref/models/querysets/#delete - it raises a `Cannot use 'limit' or 'offset' with delete` error. I was hoping that since '09 something had changed but the "for" solution still seems to be the only one! – Stefano Jan 12 '12 at 12:06

3 Answers3

53

This is a bit old, but I believe you can do the following:

notes = Notification.objects.filter(user=self.user)[:4]
Notification.objects.exclude(pk__in=list(notes)).delete()  # list() forces a database hit.

It costs two hits, but avoids using the for loop with transactions middleware.

The reason for using list(notes) is that Django creates a single query without it and, in Mysql 5.1, this raises the error

(1235, "This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'")

By using list(notes), we force a query of notes, avoiding this. This can be further optimized to:

notes = Notification.objects.filter(user=self.user)[:4].values_list("id", flat=True)  # only retrieve ids.
Notification.objects.exclude(pk__in=list(notes)).delete()
Jorge Leitao
  • 19,085
  • 19
  • 85
  • 121
  • I don't even have a django environment anymore, but I believe that this is a better solution than my for loop. – Brandon Henry Aug 18 '14 at 12:46
  • 1
    On a larger dataset most likely the better solution but the for loop is easier to read in my opinion. – radtek Aug 26 '14 at 13:26
12

Use an inner query to get the set of items you want to keep and then filter on them.

objects_to_keep = Notification.objects.filter(user=user).order_by('-created_at')[:5]
Notification.objects.exclude(pk__in=objects_to_keep).delete()

Double check this before you use it. I have found that simpler inner queries do not always behave as expected. The strange behavior I have experienced has been limited to querysets that are just an order_by and a slice. Since you will have to filter on user, you should be fine.

istruble
  • 13,363
  • 2
  • 47
  • 52
  • i tried a method similar to this and it was quirky. when it got to a certain point it deleted all the objects with the user. i believe it has something to do with how slicing works with lazy querysets. – Brandon Henry Dec 05 '09 at 19:31
  • Add the code that you tried. I would appreciate more data points for the quirky behavior. For me, the order_by seemed to be stripped off the inner query if there was no filter(). Did you experience strange behavior with an Model.objects.filter(...).orber_by(...)[:5] inner query? Oh, and you might find .query.as_sql() helpful. – istruble Dec 05 '09 at 19:48
  • i've edited my question with the code i tried. i didn't have the code which deleted all the objects, but i tried my code above (even more similar to yours), and that didn't work as i would have expected. – Brandon Henry Dec 07 '09 at 03:50
  • I like to be very explicit on things when trying to track down a problem. You might want to be explicit and add and order_by('-created_at') to your inner query. Also, you can also try looking at the raw sql with .query.as_sql(). `.delete()` does not have a .query property so remove it and add .query.as_sql() to see what is going on. Oh, probably easiest to do this in ./manage.py shell. – istruble Dec 07 '09 at 17:29
  • 3
    I can add that in MySQL 5.1 you can't do this: django.db.utils.DatabaseError: (1235, "This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'") – Emil Stenström May 28 '10 at 18:11
11

this is how i ended up doing this.

    notes = Notification.objects.filter(user=self.user)
    for note in notes[4:]:
        note.delete()

because i'm doing this in the save method, the only way the loop would ever have to run more than once would be if the user got multiple notifications at once. i'm not worried about that happening (while it may happen it's not likely to be enough to cause a problem).

Brandon Henry
  • 3,632
  • 2
  • 25
  • 30
  • 1
    This is perfectly valid. And you are already aware of the fact that you will get a sql operation for each of those deletes inside the for loop. Thanks for the update. – istruble Dec 05 '09 at 19:56
  • 12
    Breaking out the above few lines on a method and adding a commit_on_success decorator around it makes sure it stays fast even though there's many queries to be made. – Emil Stenström May 28 '10 at 18:21
  • This is a great scalable answer – Anupam Mar 12 '20 at 06:52