Tuple lookups cannot handle right-hand sides mixing literal values and expressions

[Jacob Walls]

For this model in the test suite:

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:

tests/composite_pk/test_update.py

@@ -191 +191 @@
         )
         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()

======================================================================
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().

[Simon Charette]

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 even when composite primary keys are not involved.

Take the following model for example

class Foo(models.Model):
    pass

Performing the following

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

-- 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

>>> 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

>>> 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 ​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 ​doing the equivalent) by doing

tests/composite_pk/test_filter.py

@@ -9 +9 @@
     Q,
     Subquery,
     TextField,
+    Value,
     When,
 )
 from django.db.models.functions import Cast
@@ -549 +550 @@
                 [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

django/db/models/fields/tuple_lookups.py

@@ -103 +103 @@
     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?

[Jacob Walls]

Absolutely. Was really only expecting that this low-level failure might reveal something deeper amiss, so thanks for identifying it so quickly!

[Simon Charette]

Accepting in this case, thanks for your continued iron out of the composite primary key feature.

[nessita <124304+nessita@…>]

In 0a4999b4:

Fixed #36522 -- Added support for filtering composite pks using a tuple of expressions.

Thanks Jacob Walls for the report, and Sarah Boyce and Mariusz Felisiak
for reviews.

[Natalia <124304+nessita@…>]

In 3031c51:

[5.2.x] Fixed #36522 -- Added support for filtering composite pks using a tuple of expressions.

Thanks Jacob Walls for the report, and Sarah Boyce and Mariusz Felisiak
for reviews.

Backport of 0a4999b422702c64e21f5a10a4d60300b7074401 from main.