Custom field without get_db_prep_value() errors on updates if part of composite primary key

[Jacob Walls]

A model with a custom field only implementing get_db_prep_save() but not get_db_prep_value() fails on save() when used in a composite primary key, but only for updates. Inserts work fine.

This fell out of #36062 where the test model I adjusted happened to have a ​custom field with these characteristics.

I didn't get as far as verifying this to be relevant, but I noticed while stepping through the code that SQLInsertCompiler has ​logic to add `pk` to the list of fields, so I wonder if it's not being prepared the same way in SQLUpdateCompiler?

tests/custom_pk/fields.py

@@ -59 +59 @@
         return value
 
 
+class MyWrapperFieldWithoutGetDbPrepValue(models.CharField):
+    def to_python(self, value):
+        if not value:
+            return
+        if not isinstance(value, MyWrapper):
+            value = MyWrapper(value)
+        return value
+
+    def from_db_value(self, value, expression, connection):
+        if not value:
+            return
+        return MyWrapper(value)
+
+    def get_db_prep_save(self, value, connection):
+        if not value:
+            return
+        if isinstance(value, MyWrapper):
+            return str(value)
+        return value
+
+
 class MyAutoField(models.BigAutoField):
     def from_db_value(self, value, expression, connection):
         if value is None:

tests/custom_pk/models.py

@@ -7 +7 @@
 
 from django.db import models
 
-from .fields import MyAutoField, MyWrapperField
+from .fields import MyAutoField, MyWrapperField, MyWrapperFieldWithoutGetDbPrepValue
 
 
 class Employee(models.Model):
@@ -40 +40 @@
 
 class CustomAutoFieldModel(models.Model):
     id = MyAutoField(primary_key=True)
+
+
+class CompositePKWithoutGetDbPrepValue(models.Model):
+    pk = models.CompositePrimaryKey("id", "tenant")
+    id = MyWrapperFieldWithoutGetDbPrepValue()
+    tenant = models.SmallIntegerField()

tests/custom_pk/tests.py

@@ -2 +2 @@
 from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature
 
 from .fields import MyWrapper
-from .models import Bar, Business, CustomAutoFieldModel, Employee, Foo
+from .models import Bar, Business, CompositePKWithoutGetDbPrepValue, CustomAutoFieldModel, Employee, Foo
 
 
 class BasicCustomPKTests(TestCase):
@@ -164 +164 @@
         Business.objects.create(name="Bears")
         Business.objects.create(pk="Tears")
 
+    def test_custom_composite_pk_save_force_update(self):
+        """A custom primary key field that implements get_db_prep_save()
+        rather than get_db_prep_value() still saves."""
+        obj = CompositePKWithoutGetDbPrepValue()
+        obj.id = MyWrapper("1")
+        obj.tenant = 1
+        obj.save()  # passes with force_insert=True
+
     def test_unicode_pk(self):
         # Primary key may be Unicode string.
         Business.objects.create(name="jaźń")

---

ERROR: test_custom_composite_pk_save_force_update (custom_pk.tests.CustomPKTests.test_custom_composite_pk_save_force_update)
A custom primary key field that implements get_db_prep_save()
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/.../django/django/db/backends/utils.py", line 105, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/.../django/django/db/backends/sqlite3/base.py", line 360, in execute
    return super().execute(query, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.ProgrammingError: Error binding parameter 3: type 'MyWrapper' is not supported

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/.../django/tests/custom_pk/tests.py", line 173, in test_custom_composite_pk_save_force_update
    bar.save()  # passes with force_insert=True
    ^^^^^^^^^^
...

  File "/Users/.../django/django/db/backends/sqlite3/base.py", line 360, in execute
    return super().execute(query, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.ProgrammingError: Error binding parameter 3: type 'MyWrapper' is not supported

----------------------------------------------------------------------
Ran 16 tests in 0.011s

FAILED (errors=1, skipped=1)

[Csirmaz Bendegúz]

I don't think this is related to composite primary keys, it errors if it's a regular primary key as well

[Sarah Boyce]

Yes, it's the same for regular primary keys which makes me think whether this is not valid
It's possible that the test code from serializers that inspired this is not fully defined.
Jacob if you can add more information we can look at this again

(for reference, same thing for regular primary keys)

tests/custom_pk/fields.py

@@ -20 +20 @@
         return self.value == other
 
 
-class MyWrapperField(models.CharField):
+class MyWrapperFieldWithoutGetDbPrepValue(models.CharField):
     def __init__(self, *args, **kwargs):
         kwargs["max_length"] = 10
         super().__init__(*args, **kwargs)
@@ -51 +51 @@
             return str(value)
         return value
 
+class MyWrapperField(models.CharField):
     def get_db_prep_value(self, value, connection, prepared=False):
         if not value:
             return

tests/custom_pk/models.py

@@ -7 +7 @@
 
 from django.db import models
 
-from .fields import MyAutoField, MyWrapperField
+from .fields import MyAutoField, MyWrapperField, MyWrapperFieldWithoutGetDbPrepValue
 
 
 class Employee(models.Model):
@@ -40 +40 @@
 
 class CustomAutoFieldModel(models.Model):
     id = MyAutoField(primary_key=True)
+
+
+class PKWithoutGetDbPrepValue(models.Model):
+    id = MyWrapperFieldWithoutGetDbPrepValue(primary_key=True)

tests/custom_pk/tests.py

@@ -2 +2 @@
 from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature
 
 from .fields import MyWrapper
-from .models import Bar, Business, CustomAutoFieldModel, Employee, Foo
+from .models import Bar, Business, CustomAutoFieldModel, Employee, Foo, PKWithoutGetDbPrepValue
 
 
 class BasicCustomPKTests(TestCase):
@@ -164 +164 @@
         Business.objects.create(name="Bears")
         Business.objects.create(pk="Tears")
 
+    def test_custom_composite_pk_save_force_update(self):
+        obj = PKWithoutGetDbPrepValue()
+        obj.id = MyWrapper("1")
+        obj.save()  # passes with force_insert=True
+
     def test_unicode_pk(self):
         # Primary key may be Unicode string.
         Business.objects.create(name="jaźń")