Opened 2 years ago

Closed 2 years ago

#34015 closed New feature (fixed)

Registering lookups on relation fields should be supported.

Reported by: Thomas Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 4.1
Severity: Normal Keywords: ORM lookup
Cc: Thomas, Simon Charette, AllenJonathan Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Thomas)

Hello,

I have a model, let's call it Parent, with a field called object_id. I have another model, let's call it Child, which has a ForeignKey field called parent_object[_id] pointing to Parent.object_id. I need to do a lookup on Child where the FK starts with a certain character (it's a normalized value so, in the context of my app, it makes sense... also, I didn't design this schema and changing it is not a possibility ATM).

The problem is that if I do:

qs = Child.objects.filter(parent_object_id__startswith='c')

I get:

django.core.exceptions.FieldError: Related Field got invalid lookup: startswith

The only way I could make it work is:

qs = Child.objects.filter(parent_object__object_id__startswith='c')

but it forces a join between the table and the view and that's a no-no in my case (way too costly).

Here's the MCVE (tested on Python 3.9 + Django 4.0.7 and Python 3.10 + Django 4.1.1):

import django
django.setup()
from django.db import models


class Parent(models.Model):
    class Meta:
        app_label = 'test'

    object_id = models.CharField('Object ID', max_length=20, unique=True)


class Child(models.Model):
    class Meta:
        app_label = 'test'

    parent_object = models.ForeignKey(
        Parent, to_field='object_id', related_name='%(class)s_set', on_delete=models.CASCADE
    )


if __name__ == '__main__':
    qs = Child.objects.filter(parent_object_id__startswith='c')  # fails with `FieldError: Related Field got invalid lookup: startswith`
    qs = Child.objects.filter(parent_object__object_id__startswith='c')  # works but forces a costly join

And the error:

Traceback (most recent call last):
  File "/opt/src/orm_test.py", line 26, in <module>
    qs = Child.objects.filter(parent_object_id__startswith='c')
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1420, in filter
    return self._filter_or_exclude(False, args, kwargs)
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1438, in _filter_or_exclude
    clone._filter_or_exclude_inplace(negate, args, kwargs)
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1445, in _filter_or_exclude_inplace
    self._query.add_q(Q(*args, **kwargs))
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1532, in add_q
    clause, _ = self._add_q(q_object, self.used_aliases)
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1562, in _add_q
    child_clause, needed_inner = self.build_filter(
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1478, in build_filter
    condition = self.build_lookup(lookups, col, value)
  File "/opt/src/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1292, in build_lookup
    raise FieldError(
django.core.exceptions.FieldError: Related Field got invalid lookup: startswith

Thanks for your help,

Regards,

Change History (11)

comment:1 by Thomas, 2 years ago

Description: modified (diff)

comment:2 by Thomas, 2 years ago

Cc: Thomas added

comment:3 by Mariusz Felisiak, 2 years ago

Type: BugNew feature

Thanks for the report. Django 4.2 (cd1afd553f9c175ebccfc0f50e72b43b9604bd97) allows registering lookups per field instances, so you will be able to register __startswith for parent_object_id, e.g.

parent_field = Child._meta.get_field("parent_object_id")
with register_lookup(parent_field, StartsWith):
    Child.objects.filter(parent_object_id__startswith='c')

Duplicate of #29799.

comment:4 by Mariusz Felisiak, 2 years ago

Resolution: duplicate
Status: newclosed

comment:5 by Alex Morega, 2 years ago

Also, Thomas, it makes sense to assume there is a JOIN in the second queryset, but apparently there isn't:

>>> print(Child.objects.filter(parent_object__object_id__startswith='c').query)
SELECT "test_child"."id", "test_child"."parent_object_id" FROM "test_child" WHERE "test_child"."parent_object_id" LIKE c% ESCAPE '\'

comment:6 by Thomas, 2 years ago

@Mariusz Felisiak: Thanks for the heads-up, I hadn't found that ticket in my searches.

@Alex Morega: Thank you. I should have checked the SQL of my example for the join. I relied on my findings on my existing code base which uses parent_object__object_id__startswith to circumvent the RelatedField lookup problem. I have the join there but it must be coming from somewhere else.

comment:7 by Simon Charette, 2 years ago

This also has similarities with this very old ticket https://code.djangoproject.com/ticket/2331#comment:7 so it's likely that parent_object_id__startswith='c' was actually never supported which is surprising to me.

comment:8 by Mariusz Felisiak, 2 years ago

Cc: Simon Charette AllenJonathan added
Resolution: duplicate
Status: closednew
Summary: "Related Field got invalid lookup: startswith" on CharField ForeignKeyRegistering lookups on relation fields should be supported.
Triage Stage: UnreviewedAccepted

I noticed that registering transforms on related fields doesn't work at all as we have a guard that seems completely unnecessary. A regression test:

  • tests/queries/tests.py

    diff --git a/tests/queries/tests.py b/tests/queries/tests.py
    index 1bd72dd8b8..facf0fc421 100644
    a b class Queries4Tests(TestCase):  
    16211621            date_obj,
    16221622        )
    16231623
     1624    def test_related_transform(self):
     1625        from django.db.models.functions import ExtractYear
     1626        from django.test.utils import register_lookup
     1627
     1628        date_obj = DateTimePK.objects.create()
     1629        extra_obj = ExtraInfo.objects.create(info="extra", date=date_obj)
     1630        fk_field = ExtraInfo._meta.get_field("date")
     1631        with register_lookup(fk_field, ExtractYear):
     1632            self.assertSequenceEqual(
     1633                ExtraInfo.objects.filter(date__year=2022),
     1634                [extra_obj],
     1635            )
     1636
    16241637    def test_ticket10181(self):
    16251638        # Avoid raising an EmptyResultSet if an inner query is probably
    16261639        # empty (and hence, not executed).

We could consider this a release blocker after 10178197d57476f69688d4535e550a1ea3a5eac5 🤔.

comment:9 by Simon Charette, 2 years ago

I'm not sure I understand the rationale behind making this a release blocker as lookups on related fields were never supported? I remember having to remove this check a while ago when trying to add support for a m2m__exists lookup to work around #10060 in a feature branch.

I think this a limitation we should lift with proper test coverage but I fail to see how it relates to 10178197d57476f69688d4535e550a1ea3a5eac5

Last edited 2 years ago by Simon Charette (previous) (diff)

comment:10 by Mariusz Felisiak, 2 years ago

Has patch: set
Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

PR

Agreed, it's not a release blocker.

comment:11 by GitHub <noreply@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In ce6230a:

Fixed #34015 -- Allowed filtering by transforms on relation fields.

Note: See TracTickets for help on using tickets.
Back to Top