#28863: Regression in 2.0: an annotated then filtered db function using a Q 
object
crashes
-------------------------------------+-------------------------------------
     Reporter:  Adam (Chainz)        |                    Owner:  nobody
  Johnson                            |
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Sergey Fedoseev:

Old description:

> I noticed this when trying to test Django-MySQL against 2.0 in
> https://github.com/adamchainz/django-mysql/pull/403 . Basically there's a
> function called If() that takes a Q object, and it's tested in a variety
> of scenarios. Only one of those scenarios was crashing with Django 2.0b1
> and rc1.
>
> I found a minimal test to reproduce it in the django test suite:
>
> {{{
> diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py
> index ca331aeb03..d31935ddb4 100644
> --- a/tests/expressions/tests.py
> +++ b/tests/expressions/tests.py
> @@ -73,6 +73,12 @@ class BasicExpressionsTests(TestCase):
>              ],
>          )
>
> +    def test_filtering_on_annoate_that_uses_q(self):
> +        qs = Company.objects.annotate(
> +            name_check_len=Length(Q(name__gt='T'))
> +        ).filter(name_check_len__gt=1)
> +        list(qs)
> +
>      def test_filter_inter_attribute(self):
>          # We can filter on attribute relationships on same model obj,
> e.g.
>          # find companies where the number of employees is greater
> }}}
>
> It fails with this stacktrace:
>
> {{{
> ERROR: test_filtering_on_annoate_that_uses_q
> (expressions.tests.BasicExpressionsTests)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File
> "/Users/chainz/Documents/Projects/django/tests/expressions/tests.py",
> line 80, in test_filtering_on_annoate_that_uses_q
>     list(qs)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/query.py", line
> 272, in __iter__
>     self._fetch_all()
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/query.py", line
> 1179, in _fetch_all
>     self._result_cache = list(self._iterable_class(self))
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/query.py", line
> 54, in __iter__
>     results = compiler.execute_sql(chunked_fetch=self.chunked_fetch,
> chunk_size=self.chunk_size)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
> line 1050, in execute_sql
>     sql, params = self.as_sql()
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
> line 458, in as_sql
>     where, w_params = self.compile(self.where) if self.where is not None
> else ("", [])
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
> line 390, in compile
>     sql, params = node.as_sql(self, self.connection)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/sql/where.py",
> line 79, in as_sql
>     sql, params = compiler.compile(child)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
> line 390, in compile
>     sql, params = node.as_sql(self, self.connection)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/lookups.py",
> line 160, in as_sql
>     lhs_sql, params = self.process_lhs(compiler, connection)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/lookups.py",
> line 151, in process_lhs
>     lhs_sql, params = super().process_lhs(compiler, connection, lhs)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/lookups.py",
> line 77, in process_lhs
>     lhs = lhs.resolve_expression(compiler.query)
>   File
> "/Users/chainz/Documents/Projects/django/django/db/models/expressions.py",
> line 597, in resolve_expression
>     c.source_expressions[pos] = arg.resolve_expression(query,
> allow_joins, reuse, summarize, for_save)
> AttributeError: 'WhereNode' object has no attribute 'resolve_expression'
> }}}
>
> I then used git bisect with this test and found it bisected to:
>
> {{{
> commit bde86ce9ae17ee52aa5be9b74b64422f5219530d
> Author: Sergey Fedoseev <[email protected]>
> Date:   Sat Apr 1 18:47:49 2017 +0500
>
>     Fixed #25605 -- Made GIS DB functions accept geometric expressions,
> not only values, in all positions.
> }}}
>
> This change was the culprit, if you revert only this conditional, the
> test passes:
>
> {{{
> diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py
> index d96c4468f5..c37fcabba4 100644
> --- a/django/db/models/lookups.py
> +++ b/django/db/models/lookups.py
> @@ -76,6 +76,8 @@ class Lookup:
>
>      def process_lhs(self, compiler, connection, lhs=None):
>          lhs = lhs or self.lhs
> +        if hasattr(lhs, 'resolve_expression'):
> +            lhs = lhs.resolve_expression(compiler.query)
>          return compiler.compile(lhs)
>
>      def process_rhs(self, compiler, connection):
> }}}
>
> I'm not sure of the fix, maybe `WhereNode` should be given a
> `resolve_expression` method? Or should the change in `Lookup`'s
> `process_lhs` be more considered?

New description:

 I noticed this when trying to test Django-MySQL against 2.0 in
 https://github.com/adamchainz/django-mysql/pull/403 . Basically there's a
 function called If() that takes a Q object, and it's tested in a variety
 of scenarios. Only one of those scenarios was crashing with Django 2.0b1
 and rc1.

 I found a minimal test to reproduce it in the django test suite:

 {{{
 diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py
 index ca331aeb03..d31935ddb4 100644
 --- a/tests/expressions/tests.py
 +++ b/tests/expressions/tests.py
 @@ -73,6 +73,12 @@ class BasicExpressionsTests(TestCase):
              ],
          )

 +    def test_filtering_on_annoate_that_uses_q(self):
 +        qs = Company.objects.annotate(
 +            name_check_len=Length(Q(name__gt='T'))
 +        ).filter(name_check_len__gt=1)
 +        list(qs)
 +
      def test_filter_inter_attribute(self):
          # We can filter on attribute relationships on same model obj,
 e.g.
          # find companies where the number of employees is greater
 }}}

 It fails with this stacktrace:

 {{{
 ERROR: test_filtering_on_annoate_that_uses_q
 (expressions.tests.BasicExpressionsTests)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File
 "/Users/chainz/Documents/Projects/django/tests/expressions/tests.py", line
 80, in test_filtering_on_annoate_that_uses_q
     list(qs)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/query.py", line
 272, in __iter__
     self._fetch_all()
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/query.py", line
 1179, in _fetch_all
     self._result_cache = list(self._iterable_class(self))
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/query.py", line
 54, in __iter__
     results = compiler.execute_sql(chunked_fetch=self.chunked_fetch,
 chunk_size=self.chunk_size)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
 line 1050, in execute_sql
     sql, params = self.as_sql()
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
 line 458, in as_sql
     where, w_params = self.compile(self.where) if self.where is not None
 else ("", [])
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
 line 390, in compile
     sql, params = node.as_sql(self, self.connection)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/sql/where.py",
 line 79, in as_sql
     sql, params = compiler.compile(child)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/sql/compiler.py",
 line 390, in compile
     sql, params = node.as_sql(self, self.connection)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/lookups.py",
 line 160, in as_sql
     lhs_sql, params = self.process_lhs(compiler, connection)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/lookups.py",
 line 151, in process_lhs
     lhs_sql, params = super().process_lhs(compiler, connection, lhs)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/lookups.py",
 line 77, in process_lhs
     lhs = lhs.resolve_expression(compiler.query)
   File
 "/Users/chainz/Documents/Projects/django/django/db/models/expressions.py",
 line 597, in resolve_expression
     c.source_expressions[pos] = arg.resolve_expression(query, allow_joins,
 reuse, summarize, for_save)
 AttributeError: 'WhereNode' object has no attribute 'resolve_expression'
 }}}

 I then used git bisect with this test and found it bisected to
 bde86ce9ae17ee52aa5be9b74b64422f5219530d:

 {{{
 commit bde86ce9ae17ee52aa5be9b74b64422f5219530d
 Author: Sergey Fedoseev <[email protected]>
 Date:   Sat Apr 1 18:47:49 2017 +0500

     Fixed #25605 -- Made GIS DB functions accept geometric expressions,
 not only values, in all positions.
 }}}

 This change was the culprit, if you revert only this conditional, the test
 passes:

 {{{
 diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py
 index d96c4468f5..c37fcabba4 100644
 --- a/django/db/models/lookups.py
 +++ b/django/db/models/lookups.py
 @@ -76,6 +76,8 @@ class Lookup:

      def process_lhs(self, compiler, connection, lhs=None):
          lhs = lhs or self.lhs
 +        if hasattr(lhs, 'resolve_expression'):
 +            lhs = lhs.resolve_expression(compiler.query)
          return compiler.compile(lhs)

      def process_rhs(self, compiler, connection):
 }}}

 I'm not sure of the fix, maybe `WhereNode` should be given a
 `resolve_expression` method? Or should the change in `Lookup`'s
 `process_lhs` be more considered?

--

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

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.3488585ba6a65c4008ec9fe9bbcb9c84%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to