Opened 16 years ago

Closed 16 years ago

Last modified 16 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@…, Gonzalo Saavedra Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

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 16 years ago.
query.diff (1.6 KB ) - added by matehat 16 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 by matehat, 16 years ago

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.

by matehat, 16 years ago

Attachment: signals.diff added

comment:2 by matehat, 16 years ago

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 by matehat, 16 years ago

Needs documentation: set
Summary: post_update signalpost_batch_update Signal on QuerySets

comment:4 by matehat, 16 years ago

Component: UncategorizedDatabase layer (models, ORM)

comment:5 by matehat, 16 years ago

Owner: changed from nobody to matehat

by matehat, 16 years ago

Attachment: query.diff added

comment:6 by Malcolm Tredinnick, 16 years ago

Triage Stage: UnreviewedDesign 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 by matehat, 16 years ago

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 by matehat, 16 years ago

Cc: mathieu.damours@… added

comment:9 by matehat, 16 years ago

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 by Malcolm Tredinnick, 16 years ago

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 by matehat, 16 years ago

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 by Jacob, 16 years ago

Resolution: wontfix
Status: newclosed

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 by Alex Gaynor, 16 years ago

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

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

comment:14 by Gonzalo Saavedra, 16 years ago

Cc: Gonzalo Saavedra added
Note: See TracTickets for help on using tickets.
Back to Top