Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#35125 closed New feature (needsinfo)

Parent model fields are ignored when preparing the update_fields

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

Description

Hi all,

This might be an edge case but I'd like to report this functionality that might be slightly broken, as someone coming from Django 3.X to Django 4.X.

According to 4.2 the docs, using auto_now in model fields might not result in actual updates after the save() method is called.

However, the code has a piece for backward compatibility, where these fields are indeed picked up:
https://github.com/django/django/blob/4.2.3/django/db/models/query.py#L959-962

This raises the first problem, which is the inconsistency between documentation and the actual code. I appreciate the backward compatibility though, it's a lifesaver for my specific use case :)

The other issue I am having is with a potentially unusual model scheme where I have parent classes for some models. These are not abstract classes, but sorts of aggregator classes:

class A(model.Model):
    updated_at = models.DateTimeField(auto_now=True)
    something = models.BooleanField(default=True)

# in another app
class B(A):
    name = models.CharField()

Previously, when we did B.objects.update_or_create(name="x", defaults={"something": False}) it would automatically update updated_at timestamp. In 4.2 (and above, apparently) this is no longer happening. However, if updated_at was a field in the B class, updated_at would be picked up for update with the same update_or_create call. Notice I do not need to specify the updated_at field.

My workaround for now will be to override the save method of the parent classes to include the updated_at field in the update_fields list.

when line 963 of the query is called it only uses the local model fields to prepare the new update_fields. This drops any field coming from the parent class.


I'd like to propose to use self.model._meta._non_pk_concrete_field_names instead of self.model_meta.local_concrete_fields in the above line of code, or any other attribute that contains the parent fields that its better suited. If this is accepted as an issue, I can create a patch and submit it for review/approval.

Django version: 4.2.3
Python: 3.11.1

This is the first time I am contributing to Django - I apologise if there is a better way to report issues or if there are any templates to follow.

Feel free to ask any question or ask for what is not clear in this description.

Thanks in advance.

Change History (2)

comment:1 by Natalia Bidart, 6 months ago

Keywords: models removed
Resolution: needsinfo
Status: newclosed
Type: UncategorizedNew feature

Hello Gustavo! Thanks for your report.

If I understand your second part of the report correctly, your issue is similar to the one described in this forum post: https://forum.djangoproject.com/t/update-or-create-behavior/25944/ (while the first post is about a new feature request, I think the underlying issue is the same and you may benefit from the posted answers). If yes, could you please add your proposal/concern to that topic? Or you could also create a new topic to present your idea, since you'll reach a wider audience and likely get extra feedback in the forum than here.  

For the first part of the report, could you please cite the exact part of the linked doc that you find confusing?

I'll close this as needsinfo for now, but feel free to comment back in the ticket with further information so we can re-triage. Thanks again!

in reply to:  1 comment:2 by Gustavo Silva, 6 months ago

Hi Natalia, thank you very much for your reply!

Replying to Natalia Bidart:

Hello Gustavo! Thanks for your report.

If I understand your second part of the report correctly, your issue is similar to the one described in this forum post: https://forum.djangoproject.com/t/update-or-create-behavior/25944/ (while the first post is about a new feature request, I think the underlying issue is the same and you may benefit from the posted answers). If yes, could you please add your proposal/concern to that topic? Or you could also create a new topic to present your idea, since you'll reach a wider audience and likely get extra feedback in the forum than here.  

Thank you for pointing me in that direction. I will take a look to that post and will try to see if it is the same problem - and being so I can propose a fix there, sure.

Replying to Natalia Bidart:

For the first part of the report, could you please cite the exact part of the linked doc that you find confusing?

I highlighted the bits that was confusing but perhaps they are only confusing when considering the release notes and/or the queryset reference. So, the model field reference says this about auto_now:

The field is only automatically updated when calling Model.save(). The field isn’t updated when making updates to other fields in other ways such as QuerySet.update(), though you can specify a custom value for the field in an update like that.

https://docs.djangoproject.com/en/4.2/ref/models/fields/#django.db.models.DateField.auto_now

But the release notes mention this, about the fields that are picked up for updates: https://docs.djangoproject.com/en/4.2/releases/4.2/#setting-update-fields-in-model-save-may-now-be-required

The docs for phttps://docs.djangoproject.com/en/5.0/ref/models/querysets/#update-or-create/ the update_or_create method also mention that after 4.2 only some fields are selected for update.

All of this in conjunction, led me to believe auto_now fields do not get picked up for updates after 4.2 but maybe it's only my interpretation, I don't know.

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