Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30826 closed Bug (fixed)

Chaining __contains lookup with JSONField key transforms crashes.

Reported by: Nic Perry Owned by: Louise Grandjonc
Component: Database layer (models, ORM) Version: 1.11
Severity: Release blocker Keywords: JSONField, contains
Cc: Simon Charette, Massimo Costa, Shaheed Haque Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider a model that looks like this:

class Event(models.Model):
    data = JSONField(default=dict)

Create an instance of this model with a nested array:

data = {
    "things": ["thing1", "thing2", "thing3"]
}
Event.objects.create(data=data)

The following lookup works in 1.11.24, but returns an error (traceback below) in 1.11.25:

Event.objects.filter(data__things__contains="thing1")

1.11.25 Traceback:

TypeError                                 Traceback (most recent call last)
/usr/lib/python3.7/site-packages/django/db/models/query.py in __repr__(self)
    224 
    225     def __repr__(self):
--> 226         data = list(self[:REPR_OUTPUT_SIZE + 1])
    227         if len(data) > REPR_OUTPUT_SIZE:
    228             data[-1] = "...(remaining elements truncated)..."

/usr/lib/python3.7/site-packages/django/db/models/query.py in __iter__(self)
    248                - Responsible for turning the rows into model objects.
    249         """
--> 250         self._fetch_all()
    251         return iter(self._result_cache)
    252 

/usr/lib/python3.7/site-packages/django/db/models/query.py in _fetch_all(self)
   1119     def _fetch_all(self):
   1120         if self._result_cache is None:
-> 1121             self._result_cache = list(self._iterable_class(self))
   1122         if self._prefetch_related_lookups and not self._prefetch_done:
   1123             self._prefetch_related_objects()

/usr/lib/python3.7/site-packages/django/db/models/query.py in __iter__(self)
     51         # Execute the query. This will also fill compiler.select, klass_info,
     52         # and annotations.
---> 53         results = compiler.execute_sql(chunked_fetch=self.chunked_fetch)
     54         select, klass_info, annotation_col_map = (compiler.select, compiler.klass_info,
     55                                                   compiler.annotation_col_map)

/usr/lib/python3.7/site-packages/django/db/models/sql/compiler.py in execute_sql(self, result_type, chunked_fetch)
    874             result_type = NO_RESULTS
    875         try:
--> 876             sql, params = self.as_sql()
    877             if not sql:
    878                 raise EmptyResultSet

/usr/lib/python3.7/site-packages/django/db/models/sql/compiler.py in as_sql(self, with_limits, with_col_aliases)
    439                 # (see docstring of get_from_clause() for details).
    440                 from_, f_params = self.get_from_clause()
--> 441                 where, w_params = self.compile(self.where) if self.where is not None else ("", [])
    442                 having, h_params = self.compile(self.having) if self.having is not None else ("", [])
    443                 result = ['SELECT']

/usr/lib/python3.7/site-packages/django/db/models/sql/compiler.py in compile(self, node, select_format)
    371             sql, params = vendor_impl(self, self.connection)
    372         else:
--> 373             sql, params = node.as_sql(self, self.connection)
    374         if select_format is FORCE or (select_format and not self.query.subquery):
    375             return node.output_field.select_format(self, sql, params)

/usr/lib/python3.7/site-packages/django/db/models/sql/where.py in as_sql(self, compiler, connection)
     77         for child in self.children:
     78             try:
---> 79                 sql, params = compiler.compile(child)
     80             except EmptyResultSet:
     81                 empty_needed -= 1

/usr/lib/python3.7/site-packages/django/db/models/sql/compiler.py in compile(self, node, select_format)
    371             sql, params = vendor_impl(self, self.connection)
    372         else:
--> 373             sql, params = node.as_sql(self, self.connection)
    374         if select_format is FORCE or (select_format and not self.query.subquery):
    375             return node.output_field.select_format(self, sql, params)

/usr/lib/python3.7/site-packages/django/contrib/postgres/lookups.py in as_sql(self, qn, connection)
      9         lhs, lhs_params = self.process_lhs(qn, connection)
     10         rhs, rhs_params = self.process_rhs(qn, connection)
---> 11         params = lhs_params + rhs_params
     12         return '%s %s %s' % (lhs, self.operator, rhs), params
     13 

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

This appears to have been introduced in 1.11.25 when addressing this ticket: https://code.djangoproject.com/ticket/30769

While it is entirely possible that this is an incorrect use of contains in this context to begin with, this is a breaking change for projects using 1.11.* with this syntax.

Attachments (1)

test-30826.diff (677 bytes ) - added by Mariusz Felisiak 5 years ago.
Regression test.

Download all attachments as: .zip

Change History (15)

comment:1 by Mariusz Felisiak, 5 years ago

Severity: NormalRelease blocker
Summary: Backwards-incompatible change to querying data within arrays in JSONFields using `contains` in 1.11.25Chaining __contains lookup with JSONField key transforms crashes.
Triage Stage: UnreviewedAccepted

Thanks for this report, yes it is a regression in the newest release.

Regression in 6c3dfba89215fc56fc27ef61829a6fff88be4abb.

by Mariusz Felisiak, 5 years ago

Attachment: test-30826.diff added

Regression test.

comment:2 by Mariusz Felisiak, 5 years ago

Cc: Simon Charette added

comment:3 by Massimo Costa, 5 years ago

Cc: Massimo Costa added

comment:5 by Roman Sichny, 5 years ago

getting the same with chained __has_key lookup (and i can assume any other chained lookups will crash)

comment:6 by Louise Grandjonc, 5 years ago

Owner: changed from nobody to Louise Grandjonc
Status: newassigned

in reply to:  5 comment:7 by Massimo Costa, 5 years ago

Replying to Roman Sichny:

getting the same with chained __has_key lookup (and i can assume any other chained lookups will crash)

just to confirm that my initial report (now closed as duplicate of this one) was about __has_key

comment:8 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Shaheed Haque, 5 years ago

Cc: Shaheed Haque added

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In cf7ef5d2:

[3.0.x] Fixed #30826 -- Fixed crash of many JSONField lookups when one hand side is key transform.

Regression in 6c3dfba89215fc56fc27ef61829a6fff88be4abb.

Backport of 7d1bf29977bb368d7c28e7c6eb146db3b3009ae7 from master

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 7d1bf299:

Fixed #30826 -- Fixed crash of many JSONField lookups when one hand side is key transform.

Regression in 6c3dfba89215fc56fc27ef61829a6fff88be4abb.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 323467e2:

[2.2.x] Fixed #30826 -- Fixed crash of many JSONField lookups when one hand side is key transform.

Regression in 6c3dfba89215fc56fc27ef61829a6fff88be4abb.

Backport of 7d1bf29977bb368d7c28e7c6eb146db3b3009ae7 from master

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 522af9d6:

[2.1.x] Fixed #30826 -- Fixed crash of many JSONField lookups when one hand side is key transform.

Regression in 6c3dfba89215fc56fc27ef61829a6fff88be4abb.

Backport of 7d1bf29977bb368d7c28e7c6eb146db3b3009ae7 from master.

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In a843a9ba:

[1.11.x] Fixed #30826 -- Fixed crash of many JSONField lookups when one hand side is key transform.

Regression in 6c3dfba89215fc56fc27ef61829a6fff88be4abb.

Backport of 7d1bf29977bb368d7c28e7c6eb146db3b3009ae7 from master.

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