Opened 3 weeks ago

Last modified 13 days ago

#37314 assigned Bug

Tuple lookup sanity checks consume iterator rhs

Reported by: Jacob Walls Owned by: Skeletor-Pirate
Component: Database layer (models, ORM) Version: 5.2
Severity: Normal Keywords: iterator, CompositePrimaryKey
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Similar in spirit to #37311 and #24719, we try to support iterators when provided as a lookup value by materializing them into a list or similar. This fiddle demonstrates that CompositePrimaryKey does not allow for this:

Notice the commented out version with return works fine:

from django.db import models

class Tenant(models.Model):
    name = models.CharField(max_length=10, default="", blank=True)


class Token(models.Model):
    pk = models.CompositePrimaryKey("tenant_id", "id")
    tenant = models.ForeignKey(Tenant, on_delete=models.CASCADE, related_name="tokens")
    id = models.SmallIntegerField()
    secret = models.CharField(max_length=10, default="", blank=True)


def search():
    # return 1, 1  # works
    yield 1, 1  # fails

def run():
    qs = Token.objects.filter(pk=search())
    print(qs)
...
  File "/usr/local/lib/python3.12/site-packages/django/db/models/sql/query.py", line 1678, in _add_q
    child_clause, needed_inner = self.build_filter(
                                 ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/models/sql/query.py", line 1588, in build_filter
    condition = self.build_lookup(lookups, col, value)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/models/sql/query.py", line 1415, in build_lookup
    lookup = lookup_class(lhs, rhs)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/models/lookups.py", line 38, in __init__
    self.rhs = self.get_prep_lookup()
               ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/models/fields/tuple_lookups.py", line 44, in get_prep_lookup
    self.check_rhs_length_equals_lhs_length()
  File "/usr/local/lib/python3.12/site-packages/django/db/models/fields/tuple_lookups.py", line 61, in check_rhs_length_equals_lhs_length
    raise ValueError(
ValueError: 'exact' lookup of 'pk' must have 2 elements

Setting to Django Sprints to allow some time for this to be assigned to someone participating in a mentorship program, but otherwise, in a few weeks we can unassign it.

Change History (6)

comment:1 by Zubair Hassan, 2 weeks ago

Owner: changed from Django Sprints to Zubair Hassan

comment:2 by Zubair Hassan, 2 weeks ago

For CompositePrimaryKey, the correct rhs shape for an exact match is a flat tuple: (1, 1). But your search() generator yields one item, and that one item is itself the tuple (1, 1). So materializing the generator produces [(1, 1)] — a list containing one 2-tuple — not (1, 1) directly. TupleLookupMixin.check_rhs_length_equals_lhs_length() then sees a container of length 1 where it expects length 2 (matching the two underlying fields, tenant_id and id), and raises ValueError: 'exact' lookup of 'pk' must have 2 elements.

get_prep_lookup before any of code runs, self.rhs is already [(1, 1)] generator was consumed before this function was ever called, somewhere further up the call stack.

This is how we can fix in get_prep_lookup.

if self.lookup_name == "exact" and isinstance(self.rhs, (tuple, list)) and len(self.rhs) == 1:
    self.rhs = self.rhs[0]

comment:3 by Zubair Hassan, 2 weeks ago

Has patch: set

comment:4 by Skeletor-Pirate, 2 weeks ago

Owner: changed from Zubair Hassan to Skeletor-Pirate

comment:5 by Zubair Hassan, 2 weeks ago

Triage Stage: UnreviewedAccepted

comment:6 by Jacob Walls, 13 days ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top