'exists' parameter for pre_save signal

5 views
Skip to first unread message

George Sakkis

unread,
Jun 15, 2010, 11:46:16 AM6/15/10
to Django developers
Apologies if this has come up before, I didn't find anything related
in the archives.

As Model.save() is called both for inserts and updates, it would
sometimes be useful for pre_save listeners to know whether the
instance exists or not. This information is sent with post_save but by
then it may be too late (e.g. under some conditions you may want to
prevent an insertion from happening). In the current save_base()
implementation, pre_save is sent before the instance's existence is
determined, so I'm not sure if postponing it would break existing
code. If there is no backwards compatibility issue, I can open a
ticket and send a patch for it.

George

Jeremy Dunck

unread,
Jun 15, 2010, 10:51:24 PM6/15/10
to django-d...@googlegroups.com
On Tue, Jun 15, 2010 at 10:46 AM, George Sakkis <george...@gmail.com> wrote:
> ...In the current save_base()

> implementation, pre_save is sent before the instance's existence is
> determined, so I'm not sure if postponing it would break existing
> code. If there is no backwards compatibility issue, I can open a
> ticket and send a patch for it.

Well, a significant difference is that pre_save right now can cancel a
save before a query to determine existence is needed (0 vs 1 or 2
queries). I'm not sure how many people really use pre_save's
cancellation feature, but I think we'd need some confidence that
moving wouldn't cause a problem for people counting on the 0-query
cancellation that's available right now.

George Sakkis

unread,
Jun 16, 2010, 5:30:13 AM6/16/10
to Django developers
On Jun 16, 4:51 am, Jeremy Dunck <jdu...@gmail.com> wrote:
I see how this might be an issue for some, but my guess/belief is that
often the existence or not of the instance is necessary to determine
whether the save should be canceled. Besides, the incurred cost is at
most one exists() query, which in 1.2 at least is quite lightweight.

In any case, I opened a ticket and attached a patch (with updated
tests) at http://code.djangoproject.com/ticket/13772.

George
Reply all
Reply to author
Forward
0 new messages