[Django] #31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of overridden lookup

64 views
Skip to first unread message

Django

unread,
Jul 26, 2020, 12:50:35 AM7/26/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The previous implementation of `django.contrib.postgres.fields.JSONField`
uses the overridden `contains` lookup that is JSON-based and not the
builtin one that is pattern-based.

Using the example model in the 3.1 release notes,

{{{
class ContactInfo(models.Model):
data = models.JSONField()

obj = ContactInfo.objects.create(data={
'name': 'John',
'cities': ['London', 'Cambridge'],
'pets': {'dogs': ['Rufus', 'Meg']},
})
ContactInfo.objects.filter(
data__cities__contains='Cambridge',
)
}}}

The query returns a queryset with `obj` in it, which is expected.

However, the following query:

{{{
ContactInfo.objects.filter(
data__cities__contains='bridge',
)
}}}

Also returns a queryset with `obj` in it. Using the previous
implementation, `obj` doesn't match the query because `contains` uses
JSON-based containment checking. That is, it checks whether the array in
`data__cities` contains an element that's exactly `"bridge"`.

--
Ticket URL: <https://code.djangoproject.com/ticket/31829>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 26, 2020, 12:51:25 AM7/26/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by sage:

Old description:

> The previous implementation of `django.contrib.postgres.fields.JSONField`
> uses the overridden `contains` lookup that is JSON-based and not the
> builtin one that is pattern-based.
>
> Using the example model in the 3.1 release notes,
>
> {{{
> class ContactInfo(models.Model):
> data = models.JSONField()
>
> obj = ContactInfo.objects.create(data={
> 'name': 'John',
> 'cities': ['London', 'Cambridge'],
> 'pets': {'dogs': ['Rufus', 'Meg']},
> })
> ContactInfo.objects.filter(
> data__cities__contains='Cambridge',
> )
> }}}
>
> The query returns a queryset with `obj` in it, which is expected.
>
> However, the following query:
>
> {{{
> ContactInfo.objects.filter(
> data__cities__contains='bridge',
> )
> }}}
>
> Also returns a queryset with `obj` in it. Using the previous
> implementation, `obj` doesn't match the query because `contains` uses
> JSON-based containment checking. That is, it checks whether the array in
> `data__cities` contains an element that's exactly `"bridge"`.

New description:

The previous implementation of `django.contrib.postgres.fields.JSONField`,
when chained with a `KeyTransform` and a `contains` lookup, uses the


overridden `contains` lookup that is JSON-based and not the builtin one
that is pattern-based.

Using the example model in the 3.1 release notes,

{{{
class ContactInfo(models.Model):
data = models.JSONField()

obj = ContactInfo.objects.create(data={
'name': 'John',
'cities': ['London', 'Cambridge'],
'pets': {'dogs': ['Rufus', 'Meg']},
})
ContactInfo.objects.filter(
data__cities__contains='Cambridge',
)
}}}

The query returns a queryset with `obj` in it, which is expected.

However, the following query:

{{{
ContactInfo.objects.filter(
data__cities__contains='bridge',
)
}}}

Also returns a queryset with `obj` in it. Using the previous
implementation, `obj` doesn't match the query because `contains` uses
JSON-based containment checking. That is, it checks whether the array in
`data__cities` contains an element that's exactly `"bridge"`.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:1>

Django

unread,
Jul 26, 2020, 12:52:21 AM7/26/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: assigned

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by sage):

* owner: nobody => sage
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:2>

Django

unread,
Jul 26, 2020, 2:24:24 AM7/26/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by sage):

* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/13238 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:3>

Django

unread,
Jul 27, 2020, 3:57:47 AM7/27/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* needs_better_patch: 0 => 1
* version: master => 3.1
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:4>

Django

unread,
Jul 28, 2020, 7:10:54 AM7/28/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:5>

Django

unread,
Jul 28, 2020, 2:53:07 PM7/28/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: closed

Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"2d8dcba03aae200aaa103ec1e69f0a0038ec2f85" 2d8dcba0]:
{{{
#!CommitTicketReference repository=""
revision="2d8dcba03aae200aaa103ec1e69f0a0038ec2f85"
Fixed #31829 -- Used JSONField __contains lookup on key transforms.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:6>

Django

unread,
Jul 30, 2020, 12:38:22 AM7/30/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"184a6eebb0ef56d5f1b1315a8e666830e37f3f81" 184a6eeb]:
{{{
#!CommitTicketReference repository=""
revision="184a6eebb0ef56d5f1b1315a8e666830e37f3f81"
Refs #31829 -- Added
DatabaseFeatures.json_key_contains_list_matching_requires_list.

CockroachDB's behavior matches PostgreSQL.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:8>

Django

unread,
Jul 30, 2020, 12:41:23 AM7/30/20
to django-...@googlegroups.com
#31829: Chaining KeyTransform with 'contains' lookup uses builtin lookup instead of
overridden lookup
-------------------------------------+-------------------------------------
Reporter: sage | Owner: sage
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"32cb1fe1c66645671681407d47e8370b4afbfc3f" 32cb1fe1]:
{{{
#!CommitTicketReference repository=""
revision="32cb1fe1c66645671681407d47e8370b4afbfc3f"
[3.1.x] Refs #31829 -- Added
DatabaseFeatures.json_key_contains_list_matching_requires_list.

CockroachDB's behavior matches PostgreSQL.

Backport of 184a6eebb0ef56d5f1b1315a8e666830e37f3f81 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31829#comment:9>

Reply all
Reply to author
Forward
0 new messages