Opened 5 years ago

Closed 5 years ago

Last modified 2 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: 1.6
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 (6)

comment:1 Changed 5 years ago by Tim Graham

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 Changed 5 years ago by Tim Graham

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 Changed 4 years ago by Nate Pinchot

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 Changed 4 years ago by Tim Graham

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).

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

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 Changed 2 years ago by Dylan Young

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 :(

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