Re: [Django] #37238: 6.1: save() falls back to python default for primary key that should come from related instance instead (was: 6.1: save() falls back to db_default for primary key that should come from related instance instead)

[Django]

#37238: 6.1: save() falls back to python default for primary key that should come
from related instance instead
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: 6.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* summary:
6.1: save() falls back to db_default for primary key that should come
from related instance instead
=>
6.1: save() falls back to python default for primary key that should
come from related instance instead


Old description:

> When saving an object whose primary key is a `OneToOneField`, and when
> that value is meant to come from an unsaved related object,
> a2348c85fc6c20087935c74cd99340dd4ef2dcdc has the unintentional effect of
> falling back to the field's `db_default` instead of getting it from the
> related object once saved.
>
> To demonstrate, adjust a test being drafted in this
> [https://github.com/django/django/pull/21677 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.
>
> {{{#!diff
> diff --git a/tests/bulk_create/models.py b/tests/bulk_create/models.py
> index bc9beba990..34ea0bf3f8 100644
> --- a/tests/bulk_create/models.py
> +++ b/tests/bulk_create/models.py
> @@ -157,3 +157,22 @@ class DbDefaultPrimaryKey(models.Model):
>
> 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"}
> diff --git a/tests/bulk_create/tests.py b/tests/bulk_create/tests.py
> index 397fcb9186..2692d62fb1 100644
> --- a/tests/bulk_create/tests.py
> +++ b/tests/bulk_create/tests.py
> @@ -27,6 +27,7 @@ from .models import (
> DbDefaultModel,
> DbDefaultPrimaryKey,
> FieldsWithDbColumns,
> + IntegerDbDefaultPrimaryKey,
> NoFields,
> NullableFields,
> Pizzeria,
> @@ -35,6 +36,7 @@ from .models import (
> ProxyMultiProxyCountry,
> ProxyProxyCountry,
> RelatedModel,
> + RelatedToDbDefaultPrimaryKey,
> Restaurant,
> SmallAutoFieldModel,
> State,
> @@ -439,6 +441,19 @@ class BulkCreateTests(TestCase):
> 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 = (
> }}}
>
> {{{#!py
> ======================================================================
> 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)
> }}}

New description:

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.

To demonstrate, adjust a test being drafted in this
[https://github.com/django/django/pull/21677 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.

{{{#!diff
diff --git a/tests/bulk_create/models.py b/tests/bulk_create/models.py
index bc9beba990..34ea0bf3f8 100644
--- a/tests/bulk_create/models.py
+++ b/tests/bulk_create/models.py
@@ -157,3 +157,22 @@ class DbDefaultPrimaryKey(models.Model):

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"}
diff --git a/tests/bulk_create/tests.py b/tests/bulk_create/tests.py
index 397fcb9186..2692d62fb1 100644
--- a/tests/bulk_create/tests.py
+++ b/tests/bulk_create/tests.py
@@ -27,6 +27,7 @@ from .models import (
DbDefaultModel,
DbDefaultPrimaryKey,
FieldsWithDbColumns,
+ IntegerDbDefaultPrimaryKey,
NoFields,
NullableFields,
Pizzeria,
@@ -35,6 +36,7 @@ from .models import (
ProxyMultiProxyCountry,
ProxyProxyCountry,
RelatedModel,
+ RelatedToDbDefaultPrimaryKey,
Restaurant,
SmallAutoFieldModel,
State,
@@ -439,6 +441,19 @@ class BulkCreateTests(TestCase):
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 = (
}}}

{{{#!py
======================================================================
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)
}}}

--
--
Ticket URL: <https://code.djangoproject.com/ticket/37238#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.