I changed the variable name as it made it much easier for me to reason about the origin of the f.generated and (pk_set or f is not meta.auto_field) check.

This preserves backward compatiblity for auto fields that have literal values overrides assigned to them transit through the database and be assigned back while avoiding to do so for other db_returning fields.

The reason behind it is that otherwise on backends that don't support RETURNING (MySQL) assigning an explicit literal value to a db_default field still forces a refresh from the database which is not desired.

See the following failures on MySQL to get a better understanding of why this is important

• field_defaults.tests.DefaultTests.test_null_db_default
• file_storage.tests.FileFieldStorageTests.test_files

The logic added here maintains the symmetry with the insert_fields filter criteria.

Just so my understanding is clear, we intentionally treat only auto fields here, and not other custom db_returning fields?

Editing this test like so passes on 5.2.x but fails on both main and this patch:

diff --git a/tests/queries/test_db_returning.py b/tests/queries/test_db_returning.py
index 06efe023e1..9954fa836c 100644
--- a/tests/queries/test_db_returning.py
+++ b/tests/queries/test_db_returning.py
@@ -22,7 +22,7 @@ class ReturningValuesTests(TestCase):
         )
 
     def test_insert_returning_non_integer(self):
-        obj = NonIntegerPKReturningModel.objects.create()
+        obj = NonIntegerPKReturningModel.objects.create(pk='2025-01-01')
         self.assertTrue(obj.created)
         self.assertIsInstance(obj.created, datetime.datetime)

If we're okay with this failure, then I won't add any further test edits and will just adjust docstring & commit to say "auto" fields.

Just so my understanding is clear, we intentionally treat only auto fields here, and not other custom db_returning fields?

Correct, this is an artifact of how we've treated auto-field over the years; we've always used RETURNING or (last_insert_id on MySQL) and passed the value through AutoField.from_db_value which triggers a type conversion.

Editing this test like so passes on 5.2.x but fails on both main and this patch:

I was about to say I would expect it to fail on MySQL but the test is skipped due to can_return_columns_from_insert.

I guess we could check against against field in pk_fields instead of is meta.auto_field if we want to preserve this behavior but it's venturing in undefined behavior territory IMO.

Another alternative is to always do a database RETURNING roundtrip if the backend supports it. I'll push a commit that does just that.

@jacobtylerwalls I ended up not pushing the change as to avoid stepping on your ongoing work here's what I had in mind

diff --git a/django/db/models/base.py b/django/db/models/base.py
index 3aaca16bbe..aaf051d403 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -1148,6 +1148,9 @@ def _save_table(
                 if not f.generated and (pk_set or f is not meta.auto_field)
             ]
             returning_fields = list(meta.db_returning_fields)
+            can_return_columns_from_insert = connections[
+                using
+            ].features.can_return_columns_from_insert
             for field in insert_fields:
                 value = (
                     getattr(self, field.attname) if raw else field.pre_save(self, False)
@@ -1155,7 +1158,7 @@ def _save_table(
                 if hasattr(value, "resolve_expression"):
                     if field not in returning_fields:
                         returning_fields.append(field)
-                elif field.db_returning and not (pk_set and field is meta.auto_field):
+                elif field.db_returning and not can_return_columns_from_insert:
                     returning_fields.remove(field)
             results = self._do_insert(
                 cls._base_manager, using, insert_fields, returning_fields, raw
diff --git a/tests/queries/test_db_returning.py b/tests/queries/test_db_returning.py
index 06efe023e1..2a26e6384f 100644
--- a/tests/queries/test_db_returning.py
+++ b/tests/queries/test_db_returning.py
@@ -22,7 +22,7 @@ def test_insert_returning(self):
         )

     def test_insert_returning_non_integer(self):
-        obj = NonIntegerPKReturningModel.objects.create()
+        obj = NonIntegerPKReturningModel.objects.create(pk="2025-01-01")
         self.assertTrue(obj.created)
         self.assertIsInstance(obj.created, datetime.datetime)

I exploded this out into a test case so that if we yank it later for being "dubious" we can. Should I add a little comment that this usage is a little dubious? 🤔

I was accustomed to seeing UUIDFields used like this with string representations on queryset-level operations (although not in a db_returning context) so maybe this is fine as is without a comment.

I mean, it's always dubious to make ORM calls with the unexpected type (in the sense that it's not the type to_python and full_clean would provide if called) but since the lax typing enforcement has likely ossified in code bases it's more of an artifact of some database backends accepting the string representation of dates, uuid, and others as input that we can't pull the plug on.

Yeah, the pattern:

def view(request, book_id: str):
    obj = get_object_or_404(Book, id=book_id)

…is super-common and relies on the database casting string to ID. I agree we can probably never remove this behaviour.

I blogged about one case where type conversion caused an outage, and some investigation across databases: https://adamj.eu/tech/2020/03/06/sql-implicit-type-conversion/