Opened 10 years ago

Closed 10 years ago

Last modified 4 years ago

#22981 closed Bug (wontfix)

Fields with auto_now=True are not updated if the field name is not present in update_fields list passed into Model.save

Reported by: Josh Crompton Owned by: nobody
Component: Database layer (models, ORM) Version: 3.1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Passing update_fields into a model's save method fails to update fields on the model which have auto_now=True specified but which are not included in the update_fields parameter.

I think it's a fairly common use case to have a base model which records when the instance was last updated:

Code highlighting:

  class MyModel(models.Model):
      updated_at = models.DateTimeField(auto_now=True)

But, now I have to remember to always include updated_at in my list of updated_fields:

Code highlighting:

In [2]: m = MyModel.objects.latest('updated_at')

In [3]: m.updated_at
Out[3]: datetime.datetime(2014, 7, 9, 1, 12, 12, 850847, tzinfo=<UTC>)

In [4]: m.save()

In [5]: m.updated_at
Out[5]: datetime.datetime(2014, 7, 9, 1, 12, 12, 850847, tzinfo=<UTC>)

In [6]: m.save(update_fields=['updated_at'])

In [7]: m.updated_at
Out[7]: datetime.datetime(2014, 7, 9, 2, 21, 48, 412890, tzinfo=<UTC>)

So if I have an instance of a subclass of this model and only want to update some unrelated field on that subclass, then I have to a) know that it inherits from MyModel and b) know that MyModel has a field with auto_now=True and what that field's name is and c) remember to include that in the updated_fields list whenever I use it. This seems like a lot of overhead!

Ticket #15566 addresses a similar problem (but covers the case of using Manager.update, rather than Model.save) and was determined to be a documentation issue largely because Manager.update is for updating multiple rows with a single SQL statement whereas auto_now and friends involve per-row calculations.

Since Model.save is only updating a single row, that argument doesn't hold in this case, so I'm inclined to think this is a bug rather than a documentation issue.

Change History (8)

comment:1 by Tim Graham, 10 years ago

What you consider a bug, others may consider a feature, e.g. using update_fields to bypass updating fields with auto_now. In fact, I wouldn't expect auto_now fields to be updated if not present in update_fields.

I tend to think we won't make the change due to backwards compatibility, but I will leave open for a second opinion. We can at least document the caveat.

comment:2 by Tim Graham, 10 years ago

Resolution: wontfix
Status: newclosed

From #22995 comment 11:

"I think we should at least deprecate (auto_now(_add)) because the use case is very obvious but auto_now(_add) don't implement it — and maybe can't!
Creation and modification timestamps are expected to be set automatically, regardless of how you access the row. Rails does this well at the database level."

If you are looking for the behavior you describe, try adding a database default.

comment:3 by Nate Pinchot, 9 years ago

I also would like the ability for auto_now fields to always be updated even if update_fields is used and they are not passed in. I agree that the current behavior of the way update_fields interacts with auto_now should stay as is.

Would you accept a patch which implements a new auto_now_always parameter which would provide the requested behavior of automatically pushing these fields into update_fields if not passed in?

comment:4 by Tim Graham, 9 years ago

Is there a problem with the suggestion of using a database default for your use case? I don't think more "auto" options are desired considering there has been some though to deprecate them (#22995).

in reply to:  4 comment:5 by Nate Pinchot, 9 years ago

Replying to timgraham:

Is there a problem with the suggestion of using a database default for your use case? I don't think more "auto" options are desired considering there has been some though to deprecate them (#22995).

Forgive my ignorance if I'm wrong, but my understanding is a database default won't mimic 'auto_now' behavior since it would only default on INSERT and won't change the value on UPDATE. It seems I would need a trigger to mimic the 'auto_now' behavior. I know MySQL also has a timestamp column type, but my understanding is this isn't the same as a datetime and won't work the same.

comment:6 by Dylan Young, 7 years ago

Any status on this?

Some thoughts:

If auto_now_add, as you're suggesting, should be implemented as a database default, then why hasn't Django done this? Since, as you say, this would break backwards compatibility, there are really two options:

1) As already suggested, an alternate kwarg--auto_now_always-- that sets the default in the database on migration (this solution is super kludgy)
2) An alternate default form--default_db-- that sets a database default. This couldn't use an arbitrary callable of course, but could use the database operations module.

auto_now doesn't work as a default and requires a database trigger, so it either needs to be implemented correctly in Django or the migration system needs to set these triggers: default_on_update perhaps?

The truth is that suggesting that someone add a database default is the same as saying: our ORM is broken, use your database... which really defeats the purpose of the ORM :(

comment:7 by Mr. Glass, 4 years ago

Version: 1.63.1

Is there an appeals process I can submit to for this ticket to be reopened? This is still an issue 6 years later. It seems like the only way to create an auto updated_at field which will work is to add a custom database trigger to each of my tables.

comment:8 by Carlton Gibson, 4 years ago

As per the Triage Flow you can mail the DevelopersMailingList explaining the issue and why it needs to be re-opened (and if you have a suggestion that addresses the issues raised all the better). If agreement is reached there we can reopen.

Note: See TracTickets for help on using tickets.
Back to Top