FileField(upload_to=...) callback no longer sees `auto_now_add ` field

[Ran Benita]

In Django 5.2 and before, accessing an auto_add_now=True DateTimeField in the upload_to callback of a FileField was possible, i.e. the field was filled with the current time. In Django 6.0, the field value is now None (the field is not null=True).

It is not a big problem for me, since I just dropped the usage of auto_add_now (I try to avoid it in new code anyway). But perhaps is can be helpful to document in the changelog.

The ​documentation of `upload_to` currently states that the PK is not available, maybe it can mention auto_add_now and friends as well.

from django.db import models

def upload_to_callback(instance, filename):
    return str(instance.created.year) # <- Crash, obj.created is None

class MyModel(models.Model):
    created = models.DateTimeField(
        auto_now_add=True,
    )
    file = models.FileField(
        max_length=9999,
        upload_to=upload_to_callback,
    )

[Nashrh Ashraf Khan]

I can confirm this issue is reproducible.

Tested behavior:

• Django 5.2.x: auto_now_add=True DateTimeField is populated during the upload_to callback.
• Django 6.0.x: the field value is None during upload_to, causing a crash when accessed.

This appears to be a regression in Django 6.0, as the same code works correctly in Django 5.2.
I will bisect to identify the commit where the behavior changed.

[Nashrh Ashraf Khan]

Approach:

The save-order change in Django 6.0 appears intentional, and upload_to
callables are executed before fields populated during Model.save()
(e.g. auto_now, auto_now_add, and the primary key) are assigned values.
Rather than changing the behavior, I plan to clarify this in the documentation
and add a note to the Django 6.0 release notes.

I will submit a documentation-focused PR reflecting this.

[Jacob Walls]

Hi, thanks for the report.

I tried with a very similar model that I already had wired up with a view:

def jtw(instance, filename):
    assert instance.created

class TempFile(models.Model):
    fileid = models.UUIDField(primary_key=True, default=uuid.uuid4)
    path = models.FileField(upload_to=jtw)
    created = models.DateTimeField(auto_now_add=True)

And testing both my view calling model.save() as well as the admin's add form, gives me AssertionError for a missing created on all versions back to 4.2.

Were you using a custom form?

I will bisect to identify the commit where the behavior changed.

Nashrh, did you get a bisect result?

Happy to take another look when we have more info to advance the investigation.

[Ran Benita]

Hi, thanks for looking.

And testing both my view calling model.save() as well as the admin's add form, gives me AssertionError for a missing created on all versions back to 4.2.

Interesting, I just tried it now with a fresh project and I don't get an assertion error on 5.2.10, but I do get it on 6.0.

I was able to bisect it to the following commit, which seems related:

commit 94680437a45a71c70ca8bd2e68b72aa1e2eff337
Author: Simon Charette <charette.s@gmail.com>
Date:   Wed Mar 19 01:39:19 2025 -0400

    Fixed #27222 -- Refreshed model field values assigned expressions on save().
    
    Removed the can_return_columns_from_insert skip gates on existing
    field_defaults tests to confirm the expected number of queries are
    performed and that returning field overrides are respected.

[Jacob Walls]

Are you using MySQL?

[Ran Benita]

My real project (where I first encountered it) is using PostgreSQL. I did the bisection using SQLite (default startproject configuration).

[Jacob Walls]

Aha! I reversed the order of my fields such that the DateTimeField precedes the FileField, like your model, and I reproduced. I'll dig a little bit more.

[Jacob Walls]

Wonderful, thanks for the bisect.

So I do think there is a regression in 94680437a45a71c70ca8bd2e68b72aa1e2eff337, in that we should call pre_save() with add=True if we know we are doing an insert like this:

django/db/models/base.py

@@ -1175 +1175 @@
             ].features.can_return_columns_from_insert
             for field in insert_fields:
                 value = (
-                    getattr(self, field.attname) if raw else field.pre_save(self, False)
+                    getattr(self, field.attname) if raw else field.pre_save(self, add=True)
                 )
                 if hasattr(value, "resolve_expression"):
                     if field not in returning_fields:

... because before this code was added, this was accomplished with add=True during SQLInsertCompiler.as_sql()...

                field_pre_save = partial(self.pre_save_val, field)
                field_values = [
                    field_prepare(field_pre_save(obj)) for obj in self.query.objs
                ]

... since pre_save_val() fathoms add:

    def pre_save_val(self, field, obj):
        """
        Get the given field's value off the given obj. pre_save() is used for
        things like auto_now on DateTimeField. Skip it if this is a raw query.
        """
        if self.query.raw:
            return getattr(obj, field.attname)
        return field.pre_save(obj, add=True)

So by not fathoming the add argument at this earlier point, we introduce a new vector for failures. I'm assuming we didn't bother with add=True given that we're throwing away the returned value, but this report points to a backward-compatibility issue. Tentatively accepting, since I'm expecting we'll get a more clear idea of the impact here when drafting a test case, and it's possible that there's some history about auto_now_add I need to read up on w/r/t pre_save().

Now, to the order-dependent thingy. It looks like you were relying on a fragile behavior. Another good reason to move off of auto_now_add, as you say. I think if we fix this regression then I'm not seeing as much of a need for a docs update. Thoughts?

[Simon Charette]

Thanks for the CC Jacob.

We should definitely pass add=True as you pointed out for correctness reasons but given we discard the values if they are not expressions this seems more like a way to avoid subtle undefined behavior breakages that we can afford but it doesn't require too much gymnastics on our end.

For example, something else that subtly changed is that we now call pre_save twice (in Model._save_table and SQLInsertCompiler.as_sql) and it would require way more invasive changes to address compared to flipping add=True to resolve this particular ticket.

I believe that landing a patch here should be seen as a way to correctly invoke the saving machinery (we've historically called pre_save with add=True) over a long term commitment over model field definition order dictating attribute presence on model instances.

[Jacob Walls]

I believe that landing a patch here should be seen as a way to correctly invoke the saving machinery (we've historically called pre_save with add=True) over a long term commitment over model field definition order dictating attribute presence on model instances.

I'm on board with that. Should we then add a caveat on pre_save() that it should be idempotent? We advertise a similar caveat on AppConfig.ready():

In the usual initialization process, the ready method is only called once by Django. But in some corner cases, particularly in tests which are fiddling with installed applications, ready might be called more than once. In that case, either write idempotent methods, or put a flag on your AppConfig classes to prevent rerunning code which should be executed exactly one time.

Then again maybe this is less important to note if we assume most use cases for pre_save() involve supplying missing values without side effects.

[Simon Charette]

Should we then add a caveat on pre_save() that it should be idempotent?

I think we can't until we adjust DateField.pre_save and DateTimeFied.pre_save to be. ​As of now they always generate a new value on call and assign it.

We could gate the auto_now_add and add generation by the lack of presence of the destination attribute on the provided model instances but we can't do that for auto_now=True as the call of the function is the signal that a new value should be generated.