Opened 2 months ago

Last modified 4 days ago

#35972 assigned Bug

Custom lookup example raises TypeError when looked up against a Subquery

Reported by: Jacob Walls Owned by: Jacob Walls
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: regex, mysql
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:19057

Description

On the forum I shared the TypeError I was getting when registering a custom lookup for a JSONField:

File models/lookups.py:13, in NotEqual.as_sql(self, compiler, connection)
     11 lhs, lhs_params = self.process_lhs(compiler, connection)
     12 rhs, rhs_params = self.process_rhs(compiler, connection)
---> 13 params = lhs_params + rhs_params
     14 return "%s <> %s" % (lhs, rhs), params

TypeError: can only concatenate list (not "tuple") to list

Since this problematic pattern is documented, we should either fix the documentation or fix the underlying reason it doesn't work. I haven't looked into whether there are backwards-compatible ways to do the latter.

Reproduction is just to follow the documented pattern and register it with a JSONField e.g. @JSONField.register_lookup, and then try to use it in an ORM query.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • For anyone except the patch author to review the patch using the patch review checklist and either mark the ticket as "Ready for checkin" if everything looks good, or leave comments for improvement and mark the ticket as "Patch needs improvement".

Change History (8)

comment:1 by Sarah Boyce, 2 months ago

Resolution: worksforme
Status: newclosed
Summary: Custom lookup example raises TypeErrorCustom lookup example raises TypeError when used on a JSONField

Testing against main with postgres 17 I don't get the TypeError but I get django.db.utils.DataError: invalid input syntax for type json (but I think that's expected as the documented example is not for JSON fields)

So far I found no issues with the documented example

This is what I have:

  • TabularUnified tests/custom_lookups/models.py

    a b class Article(models.Model):  
    1818
    1919class MySQLUnixTimestamp(models.Model):
    2020    timestamp = models.PositiveIntegerField()
     21
     22
     23class JSONModel(models.Model):
     24    value = models.JSONField()
     25
     26    class Meta:
     27        required_db_features = {"supports_json_field"}
  • TabularUnified tests/custom_lookups/tests.py

    diff --git a/tests/custom_lookups/tests.py b/tests/custom_lookups/tests.py
    index 2f4ea0a9a0..9a7d86f8f9 100644
    a b from django.core.exceptions import FieldError  
    66from django.db import connection, models
    77from django.db.models.fields.related_lookups import RelatedGreaterThan
    88from django.db.models.lookups import EndsWith, StartsWith
    9 from django.test import SimpleTestCase, TestCase, override_settings
     9from django.test import SimpleTestCase, TestCase, override_settings, skipUnlessDBFeature
    1010from django.test.utils import register_lookup
    1111from django.utils import timezone
    1212
    13 from .models import Article, Author, MySQLUnixTimestamp
     13from .models import Article, Author, MySQLUnixTimestamp, JSONModel
    1414
    1515
    1616class Div3Lookup(models.Lookup):
    class LookupTests(TestCase):  
    249249            self.assertSequenceEqual(qs1, [a1])
    250250            self.assertSequenceEqual(qs2, [a1])
    251251
     252    @skipUnlessDBFeature("supports_json_field")
     253    def test_custom_lookup_json_field(self):
     254        class NotEqual(models.Lookup):
     255            lookup_name = "ne"
     256
     257            def as_sql(self, compiler, connection):
     258                lhs, lhs_params = self.process_lhs(compiler, connection)
     259                rhs, rhs_params = self.process_rhs(compiler, connection)
     260                params = lhs_params + rhs_params
     261                return "%s <> %s" % (lhs, rhs), params
     262
     263        json_model_instance = JSONModel.objects.create(value={"test": "a"})
     264
     265        with (register_lookup(models.JSONField, NotEqual)):
     266            qs = JSONModel.objects.filter(value__ne="somevalue")
     267            self.assertSequenceEqual(qs, [json_model_instance])
     268
    252269    def test_custom_exact_lookup_none_rhs(self):
    253270        """
    254271        __exact=None is transformed to __isnull=True if a custom lookup class

comment:2 by Jacob Walls, 2 months ago

Resolution: worksforme
Status: closednew
Summary: Custom lookup example raises TypeError when used on a JSONFieldCustom lookup example raises TypeError when looked up against a Subquery

Thanks Sarah for stubbing out a test. Sorry I didn't notice the special ingredient was Subquery and not JSONField, although I take it there could be other offenders besides Subquery?

This fails:

  • TabularUnified tests/custom_lookups/tests.py

    diff --git a/tests/custom_lookups/tests.py b/tests/custom_lookups/tests.py
    index 2f4ea0a9a0..b1681621c6 100644
    a b class LookupTests(TestCase):  
    249249            self.assertSequenceEqual(qs1, [a1])
    250250            self.assertSequenceEqual(qs2, [a1])
    251251
     252    def test_custom_lookup_with_subquery(self):
     253        class NotEqual(models.Lookup):
     254            lookup_name = "ne"
     255
     256            def as_sql(self, compiler, connection):
     257                lhs, lhs_params = self.process_lhs(compiler, connection)
     258                rhs, rhs_params = self.process_rhs(compiler, connection)
     259                params = lhs_params + rhs_params
     260                return "%s <> %s" % (lhs, rhs), params
     261
     262        author = Author.objects.create(name="Isabella")
     263
     264        with register_lookup(models.Field, NotEqual):
     265            qs = Author.objects.annotate(
     266                unknown_age=models.Subquery(
     267                    Author.objects.filter(age__isnull=True).values("name")
     268                )
     269            ).filter(unknown_age__ne="Plato")
     270            self.assertSequenceEqual(qs, [author])
     271
    252272    def test_custom_exact_lookup_none_rhs(self):
    253273        """
    254274        __exact=None is transformed to __isnull=True if a custom lookup class

comment:3 by Sarah Boyce, 2 months ago

Triage Stage: UnreviewedAccepted

Thank you for the follow up Jacob 👍

comment:4 by Jacob Walls, 2 months ago

Component: DocumentationDatabase layer (models, ORM)
Keywords: regex mysql added

I originally framed it as a documentation issue, given that a cleanup/optimization to harden this is probably blocked on a DEP to type-annotate the ORM, but here is a test that fails on MariaDB using only built in lookups (almost certainly on MySQL as well), so we do have a bug in core:

  • TabularUnified tests/custom_lookups/tests.py

    diff --git a/tests/custom_lookups/tests.py b/tests/custom_lookups/tests.py
    index 2f4ea0a9a0..0fe21eb48c 100644
    a b class LookupTests(TestCase):  
    249249            self.assertSequenceEqual(qs1, [a1])
    250250            self.assertSequenceEqual(qs2, [a1])
    251251
     252    def test_regex_lookup_with_subquery(self):
     253        author = Author.objects.create(name="Isabella")
     254
     255        qs = Author.objects.annotate(
     256            unknown_age=models.Subquery(
     257                Author.objects.filter(age__isnull=True).values("name")
     258            )
     259        ).filter(name__regex=models.F("unknown_age"))
     260        self.assertSequenceEqual(qs, [author])
     261
    252262    def test_custom_exact_lookup_none_rhs(self):
    253263        """
    254264        __exact=None is transformed to __isnull=True if a custom lookup class

So I think a documentation update is still worthwhile, but after that we should leave this open until we fix the regex lookup or do that DEP :D

comment:5 by Jacob Walls, 2 months ago

Owner: set to Jacob Walls
Status: newassigned

comment:6 by Jacob Walls, 4 weeks ago

Has patch: set

comment:7 by Sarah Boyce, 7 days ago

Needs tests: set

comment:8 by Jacob Walls, 4 days ago

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