#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)
Change History (16)
comment:1 by , 16 years ago
by , 16 years ago
Attachment: | signals.diff added |
---|
comment:2 by , 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 , 16 years ago
Needs documentation: | set |
---|---|
Summary: | post_update signal → post_batch_update Signal on QuerySets |
comment:4 by , 16 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|
comment:5 by , 16 years ago
Owner: | changed from | to
---|
by , 16 years ago
Attachment: | query.diff added |
---|
comment:6 by , 16 years ago
Triage Stage: | Unreviewed → 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 by , 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 , 16 years ago
Cc: | added |
---|
comment:9 by , 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 , 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 , 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 , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | new → 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 by , 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 , 16 years ago
Cc: | added |
---|
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.