Code

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#9885 closed (wontfix)

post_batch_update Signal on QuerySets

Reported by: matehat Owned by: matehat
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: mathieu.damours@…, gonz Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Wouldn't it be nice to have a post_update signal to be triggered on every update done on a QuerySet object (since it is already implemented at model level for single-model updates). For an open project I'm preparing, I need to track changed done to registered models to that an indexing table is always up-to-date, and such signal is necessary, since update on queryset level doesn't currently trigger any project-wide event or signal. So I made the necessary changes (very small indeed) and attached the corresponding patches.

Attachments (2)

signals.diff (684 bytes) - added by matehat 6 years ago.
query.diff (1.6 KB) - added by matehat 6 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by matehat

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

As a start, I decided to provide queryset and raw argument along with the signal. raw is a reference to the keyword arguments dictionary passed to the update method and queryset is the corresponding queryset.

Changed 6 years ago by matehat

comment:2 Changed 6 years ago by matehat

Changed the names to avoid confusion with current model-level signals (new name is post_batch_update). Also added pre_batch_update, pre_batch_delete and post_batch_delete and made the necessary changes to the QuerySet methods.

comment:3 Changed 6 years ago by matehat

  • Needs documentation set
  • Summary changed from post_update signal to post_batch_update Signal on QuerySets

comment:4 Changed 6 years ago by matehat

  • Component changed from Uncategorized to Database layer (models, ORM)

comment:5 Changed 6 years ago by matehat

  • Owner changed from nobody to matehat

Changed 6 years ago by matehat

comment:6 Changed 6 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Design decision needed

Adding any new signals to Django's core is something we only do if there's a really great need. This may not necessarily cross that barrier. You can at least simulate the same effect using a custom QuerySet that emits whatever signal you like (or calls some function) when the appropriate methods are called. So adding a signal isn't required here and may not be worth the extra cost.

comment:7 Changed 5 years ago by matehat

I think I need to be clearer. I'm making a open pluggable search engine for django, that integrates very extensible search functionalities (e.g. searching into different models, annotating a relevance value and sorting accordingly, for instance). One of its facet is automatic indexing of changes that happen on indexed models. To keep the indexing tables always up-to-date and consistent, it needs to be triggered every time any kind of update is issued to the database. Requiring the users of the engine I'm making to use a subclass of Queryset I'd provide wouldn't be much appreciated I think.

IMHO I don't think triggering signals on batch- update and delete commands would cost that much, as each of them is called only once for a group of items and we already call 2 signals for every save() and delete() commands sent to particular model instances.

comment:8 Changed 5 years ago by matehat

  • Cc mathieu.damours@… added

comment:9 Changed 5 years ago by matehat

Besides, in the majority of cases, these signals won't be connected, so the added workload is barely this:

if not self.receivers:
  return []

so it really shouldn't be a concern whether added signals is that much of an extra cost. IMHO Django maintainers should include as much of these signals as possible because it allows better code on the developers side (so we don't have to override core classes, thus making our code incompatible with each other). Such design decision is about making django extensible, which is exactly what we all love about this framework.

comment:10 Changed 5 years ago by mtredinnick

I'm still very reluctant to add this signal, but I'm not going to wontfix until another developer concurs. For the record, it's not even going to be a reliable result, since the rows that are updated are decided by the database and not available to Django (whereas the single model save case knows exactly which row is updated).

Would like another core developer to sanity check before we resolve this one way or the other.

comment:11 Changed 5 years ago by matehat

Even if we can't be totally sure of the rows that are being updated or deleted, we still have a queryset in hand with the where clauses that should, in many cases, point the same rows before and after the update query, but an update query that updates the very columns that were used to build the queryset would indeed see that kind of signal useless.

comment:12 Changed 5 years ago by jacob

  • Resolution set to wontfix
  • Status changed from new to closed

I concur with Malcolm: a signal that contains no actual information about what's going to be updated will probably cause more harm than good. You'd have to introspect the queryset and actually fetch the rows, and even then you'd not necessarily get the right ones because the db could change under your feet. Worse, this signal which you might not be aware of would change a simple UPDATE to a SELECT; UPDATE which could slow things down a whole lot. Marking wontfix.

comment:13 Changed 5 years ago by Alex

Further, the queryset wouldn't even necessarily have the right items, consider:

Ticket.objects.filter(status=UNREVIEWED).update(status=CLOSED)

comment:14 Changed 5 years ago by gonz

  • Cc gonz added

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.