#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>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/010701983daffaf8-8ca444ec-6996-4ebb-8f8f-66962f3eb9e9-000000%40eu-central-1.amazonses.com.
