[Django] #35890: pre_save field in parent models are not called during update in update_or_create

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
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.

I'm also preparing a PR with a unit test and the fix, I'll update this
ticket when it will be ready.
--
Ticket URL: <https://code.djangoproject.com/ticket/35890>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------

Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Gagaro):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:1>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Gagaro:

Old description:

New description:

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

--
--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:2>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted

Comment:

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

{{{#!python
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

{{{#!python
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`
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:3>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:4>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:5>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* cc: Simon Charette, Florian Apolloner, Sarah Boyce (added)

Comment:

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
[https://forum.djangoproject.com/t/update-or-create-behavior/25944 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
([https://github.com/django/django/pull/13526 original PR by Florian],
[https://github.com/django/django/pull/16072 eventually merged changes by
Sarah]) unfortunately it didn't cover this use case,
[https://github.com/django/django/pull/13526#discussion_r503304168 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:6>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Gagaro):

Indeed, I see your point.

I didn't know that `auto_now` was supposed to be deprecated. It isn't
obvious from the
[https://docs.djangoproject.com/en/5.1/ref/models/fields/#django.db.models.DateField.auto_now
documentation] either.
--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:7>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------
Reporter: Gagaro | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Clifford Gama):

* cc: Clifford Gama (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:8>

[Django]

#35890: pre_save field in parent models are not called during update in
update_or_create
-------------------------------------+-------------------------------------

Reporter: Gagaro | Owner: Jericho
| Serrano
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jericho Serrano):

* owner: (none) => Jericho Serrano
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35890#comment:9>