pre_save field in parent models are not called during update in update_or_create

[Gagaro]

With the following models:

class Parent(models.Model):
    modified_at = models.DateTimeField(auto_now=True)


class Child(Parent):
    modified_at_child = models.DateTimeField(auto_now=True)

When calling update_or_create, the pre_save of the modified_at field is not called, and the field is not updated:

# Create

>>> instance, _ = Child.objects.update_or_create(pk=1)
>>> instance.modified_at
datetime.datetime(2024, 11, 6, 14, 1, 11, 52881, tzinfo=datetime.timezone.utc)
>>> instance.modified_at_child
datetime.datetime(2024, 11, 6, 14, 1, 11, 54363, tzinfo=datetime.timezone.utc)


# Update

>>> instance, _ = Child.objects.update_or_create(pk=1)
>>> instance.modified_at
datetime.datetime(2024, 11, 6, 14, 1, 11, 52881, tzinfo=datetime.timezone.utc)
>>> instance.modified_at_child
datetime.datetime(2024, 11, 6, 14, 1, 21, 517245, tzinfo=datetime.timezone.utc)

The regression seems to have happened in ​https://github.com/django/django/commit/6cc0f22a73970dd7c0d29d4d8d2ff9e1cc862b30

Using self.model._meta.concrete_fields instead of self.model._meta.local_concrete_fields seems to solve the issue, but I don't know the internal API well enough to understand all of the implications.

PR : ​https://github.com/django/django/pull/18777

[Simon Charette]

I see no reasons why this should only work for local concrete fields given update_or_create works with MTI.

There's a nuance not captured in the patch though that I believe should be addressed at the same time. Only pre-save fields for the model associated with at-least-one update field should be augmented. What I mean by that is that given the following models

class Location(models.Model):
    location_updated_at = models.DateTimeField(auto_now=True)
    location_street = models.TextField()

class Restaurant(Location):
    restaurant_updated_at = models.DateTimeField(auto_now=True)
    menu = models.JSONField()

Then

Restaurant.objects.update_or_create(
   ...,
   defaults={"menu": ...}  # Only restaurant_updated_at should be included in `update_fields`
)
Restaurant.objects.update_or_create(
   ...,
   defaults={"location_street": ...}  # Only location_updated_at should be included in `update_fields`
)
Restaurant.objects.update_or_create(
   ...,
   defaults={"menu": ..., "location_street": ...}  # Both restaurant_updated_at and location_updated_at should be included in `update_fields`
)

[Gagaro]

Why do you think it should be that way?

In our cases, we have an auto_now field on the parent model only, and we need it to be updated every time, even if there is only a single change in a field from a child model.

[Gagaro]

I had more thoughts about it, and from a user perspective, I think having some fields but not others updated would be an unexpected behavior. The fields on the parent model are parts of the child one, it is transparent when we try to access or change them on the instance. I'd expect them to be updated in the same way the child ones are.

[Simon Charette]

I think we need someone else to chime in here to get a consensus before moving forward with either solution.

The Field.auto_now pattern is a feature on its way out #22995 and as it doesn't play nicely with update_fields #22981 particularly in the context of update_or_create (#35014 and associated ​forum discussion).

In your particular case, Child.save(update_fields={...}), won't update "modified_at" unless it's specified explicitly so your expectations are already broken for any direct or indirect (through third-party apps) usage of save(update_fields); update_or_create is only a single instance of that.

The changes made to handle local concrete fields in #32095 (​original PR by Florian, ​eventually merged changes by Sarah) unfortunately it didn't cover this use case, ​even if it was pointed out as a point of ambiguity, as neither Book or Publisher are using MTI.

Why do you think it should be that way?

From my perspective, given the intent #32095 was to avoid unnecessary field updates, performing MTI inherited tables updates when non of their fields are explicitly requested to be updated because they use a pattern meant to be deprecated is not the right approach.

[Gagaro]

Indeed, I see your point.

I didn't know that auto_now was supposed to be deprecated. It isn't obvious from the ​documentation either.