save() falls back to python default even when DatabaseDefault instance is assigned

[Jacob Walls]

When saving an object whose primary key is a OneToOneField, and when that value is meant to come from an unsaved related object, and when that related object uses db_default, a2348c85fc6c20087935c74cd99340dd4ef2dcdc has the unintentional effect of falling back to the field's default instead of getting it from the related object once saved. EDIT: before 6.1, the fallback was to the related object's db_default expression, which isn't quite right either, so my current proposal is to just match the 6.0 behavior and then bounce the rest to a follow-up issue.

To demonstrate, adjust a test being drafted in this ​other PR (where the bulk_create() case was considered and handled) to use save() instead:

Passes before a2348c85fc6c20087935c74cd99340dd4ef2dcdc on SQLite. Other databases have other behaviors, like raising integrity errors, so we might need some test skips here. This should be rewritten to use Now() instead of a constant value; I found the constant value was easier to debug.

tests/bulk_create/models.py

@@ -157 +157 @@
 
     class Meta:
         required_db_features = {"supports_expression_defaults"}
+
+
+class IntegerDbDefaultPrimaryKey(models.Model):
+    id = models.IntegerField(primary_key=True, db_default=42)
+
+    class Meta:
+        required_db_features = {"supports_expression_defaults"}
+
+
+class RelatedToDbDefaultPrimaryKey(models.Model):
+    related = models.OneToOneField(
+        IntegerDbDefaultPrimaryKey,
+        on_delete=models.CASCADE,
+        primary_key=True,
+        default=1,
+    )
+
+    class Meta:
+        required_db_features = {"supports_expression_defaults"}

tests/bulk_create/tests.py

@@ -27 +27 @@
     DbDefaultModel,
     DbDefaultPrimaryKey,
     FieldsWithDbColumns,
+    IntegerDbDefaultPrimaryKey,
     NoFields,
     NullableFields,
     Pizzeria,
@@ -35 +36 @@
     ProxyMultiProxyCountry,
     ProxyProxyCountry,
     RelatedModel,
+    RelatedToDbDefaultPrimaryKey,
     Restaurant,
     SmallAutoFieldModel,
     State,
@@ -439 +441 @@
         child = NullableFields.objects.get(integer_field=88)
         self.assertEqual(child.auto_field, parent)
 
+    @skipUnlessDBFeature(
+        "can_return_columns_from_insert", "supports_expression_defaults"
+    )
+    def test_pk_from_related_instance_saved_after_init_with_defaults(self):
+        # Ensure that the related field's Python default references a
+        # different valid object.
+        IntegerDbDefaultPrimaryKey.objects.create(pk=1)
+        related_object = IntegerDbDefaultPrimaryKey()
+        obj = RelatedToDbDefaultPrimaryKey(related=related_object)
+        related_object.save()
+        obj.save()
+        self.assertEqual(obj.pk, related_object.pk)
+
     def test_unsaved_parent(self):
         parent = NoFields()
         msg = (

======================================================================
FAIL: test_pk_from_related_instance_saved_after_init_with_defaults (bulk_create.tests.BulkCreateTests.test_pk_from_related_instance_saved_after_init_with_defaults)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jwalls/django/tests/bulk_create/tests.py", line 455, in test_pk_from_related_instance_saved_after_init_with_defaults
    self.assertEqual(obj.pk, related_object.pk)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 1 != 42

----------------------------------------------------------------------
Ran 1 test in 0.007s

FAILED (failures=1)

[Jacob Walls]

Ah, in no recent versions does the primary key get inserted with the value from the related object. (It comes from the related object's db_default expression, which can be a separate non-release-blocker ticket.) But it at least shouldn't start coming from the python default in 6.1, I gather.

[Jacob Walls]

​PR with a more understandable test 😅

[Sarah Boyce]

The patch for #37239 appears to resolve this ticket, and the issue sounds similar, so I am going to mark as a duplicate
If there are test cases where the fix for #37239 wouldn't fix this but demonstrate this issue, we can reopen

[Jacob Walls]

Good spot, added a more distinct test case. There's also the issue of whether the other issue will be backported (probably not), but this one should be since it's a 6.1 regression.

[Jacob Walls]

Added a release note and versionchanged for the _is_pk_set() change from #37139 (good idea!). Let me know if you had something else in mind.

[Sarah Boyce <42296566+sarahboyce@…>]

In 9465349:

Fixed #37238 -- Prevented fallback to python default for a pk with a db_default.

Regression in a2348c85fc6c20087935c74cd99340dd4ef2dcdc.

[Sarah Boyce <42296566+sarahboyce@…>]

In c6e5cf9:

[6.1.x] Fixed #37238 -- Prevented fallback to python default for a pk with a db_default.

Regression in a2348c85fc6c20087935c74cd99340dd4ef2dcdc.

Backport of 9465349120ef8a0b0689e12bcbfd05f3d173ebdf from main.