[Django] #36522: Assigning expressions to model fields does not work if the field participates in a composite primary key

[Django]

#36522: Assigning expressions to model fields does not work if the field
participates in a composite primary key
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Release
| blocker
Keywords: | Triage Stage:
compositeprimarykey, F | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
For this model in the test suite:
{{{#!py
class Comment(models.Model):
pk = models.CompositePrimaryKey("tenant", "id")
tenant = models.ForeignKey(
Tenant,
on_delete=models.CASCADE,
related_name="comments",
)
id = models.SmallIntegerField(unique=True, db_column="comment_id")
...
}}}

This fails:
{{{#!diff
diff --git a/tests/composite_pk/test_update.py
b/tests/composite_pk/test_update.py
index 697383b007..e76210e75e 100644
--- a/tests/composite_pk/test_update.py
+++ b/tests/composite_pk/test_update.py
@@ -191,3 +191,7 @@ class CompositePKUpdateTests(TestCase):
)
with self.assertRaisesMessage(FieldError, msg):
Comment.objects.update(text=F("pk"))
+
+ def test_update_fields_expression(self):
+ self.comment_1.id = F("id") + 100
+ self.comment_1.save()
}}}
{{{#!py
======================================================================
ERROR: test_update_fields_expression
(composite_pk.test_update.CompositePKUpdateTests.test_update_fields_expression)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/jwalls/django/django/db/models/fields/__init__.py", line
2130, in get_prep_value
return int(value)
TypeError: int() argument must be a string, a bytes-like object or a real
number, not 'CombinedExpression'

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

Traceback (most recent call last):
File "/Users/jwalls/django/tests/composite_pk/test_update.py", line 197,
in test_update_fields_expression
self.comment_1.save()
~~~~~~~~~~~~~~~~~~~^^
File "/Users/jwalls/django/django/db/models/base.py", line 874, in save
self.save_base(
~~~~~~~~~~~~~~^
using=using,
^^^^^^^^^^^^
...<2 lines>...
update_fields=update_fields,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
File "/Users/jwalls/django/django/db/models/base.py", line 966, in
save_base
updated = self._save_table(
raw,
...<4 lines>...
update_fields,
)
File "/Users/jwalls/django/django/db/models/base.py", line 1097, in
_save_table
updated = self._do_update(
base_qs, using, pk_val, values, update_fields, forced_update
)
File "/Users/jwalls/django/django/db/models/base.py", line 1165, in
_do_update
return filtered._update(values) > 0
~~~~~~~~~~~~~~~~^^^^^^^^
File "/Users/jwalls/django/django/db/models/query.py", line 1319, in
_update
return query.get_compiler(self.db).execute_sql(ROW_COUNT)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 2103,
in execute_sql
row_count = super().execute_sql(result_type)
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 1611,
in execute_sql
sql, params = self.as_sql()
~~~~~~~~~~~^^
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 2089,
in as_sql
where, params = self.compile(self.query.where)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 578,
in compile
sql, params = node.as_sql(self, self.connection)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/sql/where.py", line 151, in
as_sql
sql, params = compiler.compile(child)
~~~~~~~~~~~~~~~~^^^^^^^
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 578,
in compile
sql, params = node.as_sql(self, self.connection)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/fields/tuple_lookups.py",
line 130, in as_sql
return super().as_sql(compiler, connection)
~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/lookups.py", line 407, in
as_sql
return super().as_sql(compiler, connection)
~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/lookups.py", line 239, in
as_sql
rhs_sql, rhs_params = self.process_rhs(compiler, connection)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/fields/tuple_lookups.py",
line 109, in process_rhs
return compiler.compile(Tuple(*args))
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 576,
in compile
sql, params = vendor_impl(self, self.connection)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/fields/tuple_lookups.py",
line 46, in as_sqlite
return self.as_sql(compiler, connection)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/expressions.py", line 1107,
in as_sql
arg_sql, arg_params = compiler.compile(arg)
~~~~~~~~~~~~~~~~^^^^^
File "/Users/jwalls/django/django/db/models/sql/compiler.py", line 576,
in compile
sql, params = vendor_impl(self, self.connection)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/expressions.py", line 29, in
as_sqlite
sql, params = self.as_sql(compiler, connection, **extra_context)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/db/models/expressions.py", line 1175,
in as_sql
val = output_field.get_db_prep_value(val, connection=connection)
File "/Users/jwalls/django/django/db/models/fields/__init__.py", line
2137, in get_db_prep_value
value = super().get_db_prep_value(value, connection, prepared)
File "/Users/jwalls/django/django/db/models/fields/__init__.py", line
1006, in get_db_prep_value
value = self.get_prep_value(value)
File "/Users/jwalls/django/django/db/models/fields/__init__.py", line
2132, in get_prep_value
raise e.__class__(
"Field '%s' expected a number but got %r." % (self.name, value),
) from e
TypeError: Field 'id' expected a number but got <CombinedExpression:
Col(composite_pk_comment, composite_pk.Comment.id) + Value(100)>.
}}}
----
The `+ 100` is not necessary to reproduce, just there for realism. Error
without is the same: "expected a number but got Col(..."

If support is difficult, a `FieldError` is perhaps better, see
`test_update_value_not_composite()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36522>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36522: Assigning expressions to model fields does not work if the field
participates in a composite primary key
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage:
compositeprimarykey, F | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* cc: Simon Charette (added)

Comment:

You can't use primary key assignment (complete or partial) to model
instances to perform an update as the value is used both for the desired
value as well as the row lookup, that is excluding composite primary keys.

Take the following model for example

{{{#!python
class Foo(models.Model):
pass
}}}

Performing the following

{{{#!python
foo = Foo.objects.create()
foo.id = F("id") + 1
foo.save()
}}}

will inevitably be a noop as the value assigned to `id` must be used for
the `SET` and `WHERE` clause so it will result in

{{{#!sql
-- This can't match any row since id = id + 1 cannot be satisfied
UPDATE foo SET id = (id + 1) WHERE id = id + 1
}}}

If you use `force_update=True` you'll get an appropriate error message

{{{#!python
>>> foo.save(force_update=True)
django.db.utils.DatabaseError: Forced update did not affect any rows.
}}}

but if you don't `save()` will attempt a subsequent `INSERT` and crash as
`F` field reference cannot be used

{{{#!python
>>> foo.save(force_update=True)
ValueError: Failed to insert expression "Col(foo, Foo.id) + Value(1)" on
Foo.id. F() expressions can only be used to update, not to insert.
}}}

In other words, you can't use `Model.save` to update a primary key you
must use `QuerySet.update` for that, maybe be could add a mention to
`Model.save`
[https://docs.djangoproject.com/en/5.2/ref/models/instances/#django.db.models.Model.save
docs] for that but given we don't support database level `ON UPDATE`
anyway I'm not convinced it's worth documenting (it's uncommon for a
primary key to be updated).

Now the crash you're getting here surfaces another issue though which is
that `TupleExact` lookup (and likely other tuple lookups) do not properly
handle right-hand-sides mixing literal and expression values. You can get
a similar crash to the one you ran into (caused by `Model.save`
[https://github.com/django/django/blob/485f483d49144a2ea5401442bc3b937a370b3ca6/django/db/models/base.py#L1143
doing the equivalent]) by doing

{{{#!diff
diff --git a/tests/composite_pk/test_filter.py
b/tests/composite_pk/test_filter.py
index 03037d4d82..4deb5f9b3b 100644
--- a/tests/composite_pk/test_filter.py
+++ b/tests/composite_pk/test_filter.py
@@ -9,6 +9,7 @@
Q,
Subquery,
TextField,
+ Value,
When,
)
from django.db.models.functions import Cast
@@ -549,6 +550,12 @@ def test_outer_ref_in_filtered_relation(self):
[self.tenant_1],
)

+ def test_filter_by_tuple_containing_expression(self):
+ qs = Comment.objects.filter(
+ pk=(self.comment_1.tenant.id, (Value(self.comment_1.id) + 1)
- 1)
+ )
+ self.assertEqual(qs.get(), self.comment_1)
+

@skipUnlessDBFeature("supports_tuple_lookups")
class CompositePKFilterTupleLookupFallbackTests(CompositePKFilterTests):
}}}

which can be solved by

{{{#!diff
diff --git a/django/db/models/fields/tuple_lookups.py
b/django/db/models/fields/tuple_lookups.py
index e929c9bb89..9d0da376f2 100644
--- a/django/db/models/fields/tuple_lookups.py
+++ b/django/db/models/fields/tuple_lookups.py
@@ -103,7 +103,11 @@ def process_lhs(self, compiler, connection,
lhs=None):
def process_rhs(self, compiler, connection):
if self.rhs_is_direct_value():
args = [
- Value(val, output_field=col.output_field)
+ (
+ val
+ if hasattr(val, "as_sql")
+ else Value(val, output_field=col.output_field)
+ )
for col, val in zip(self.lhs, self.rhs)
]
return compiler.compile(Tuple(*args))
}}}

With this patch applied your provided test fails with the exact same
signatures as the one when no composite primary keys are involved.

Should re-purpose this ticket to focus on the lack of support for `tuple:
Expression` as right-hand-side for tuple lookups?
--
Ticket URL: <https://code.djangoproject.com/ticket/36522#comment:1>