Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Release blocker  |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * has_patch:  0 => 1


-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018be0371665-c6309a4b-df39-4c7c-aba3-5dacacd5deea-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Release blocker  |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * severity:  Normal => Release blocker


Comment:

 Confirmed regression in b181cae2e3697b2e53b5b67ac67e59f3b05a6f0d, refs
 #25307.

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018be00c4f18-1b67d316-257e-43eb-aef0-c7aaac2b169e-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 I've got an idea of how to solve this, it's not clear to me if it's a
 regression as the underlying cause has little to do with window functions.

 In the mean time, can you confirm that switching the order of your lookup
 resolve the issue. That is doing `filter=Q(new_ordering=F('ordering'))`
 instead of `filter=Q(ordering=F('new_ordering'))`.

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bdf9c2a66-0105dc94-41e3-49ab-9c76-3c38dba06811-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * owner:  nobody => Simon Charette
 * status:  new => assigned


-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bdf99c4e1-4f580a20-9949-476d-acd5-9b3d3780-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Sergey Nesterenko):

 Replying to [comment:4 Simon Charette]:
 > I'm not sure if something else will break as I don't think we have
 extensive testing for performing filtered aggregation over a window
 function but does the following patch helps
 >
 > {{{#!python
 > diff --git a/django/db/models/expressions.py
 b/django/db/models/expressions.py
 > index 3a0c75ebf2..74ae9cab8e 100644
 > --- a/django/db/models/expressions.py
 > +++ b/django/db/models/expressions.py
 > @@ -417,6 +417,8 @@ def replace_expressions(self, replacements):
 >  def get_refs(self):
 >  refs = set()
 >  for expr in self.get_source_expressions():
 > +if expr is None:
 > +continue
 >  refs |= expr.get_refs()
 >  return refs
 > }}}


 I thought about it, but I wasn't sure it wouldn't break anything, but it
 might work.

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bde8126dd-fdadeef2-d87d-4fd6-88d4-9708f86d8399-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 I'm not sure if something else will break as I don't think we have
 extensive testing for performing filtered aggregation over a window
 function but does the following patch helps

 {{{#!python
 diff --git a/django/db/models/expressions.py
 b/django/db/models/expressions.py
 index 3a0c75ebf2..74ae9cab8e 100644
 --- a/django/db/models/expressions.py
 +++ b/django/db/models/expressions.py
 @@ -417,6 +417,8 @@ def replace_expressions(self, replacements):
  def get_refs(self):
  refs = set()
  for expr in self.get_source_expressions():
 +if expr is None:
 +continue
  refs |= expr.get_refs()
  return refs
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bde7cf397-96dc58b4-79f8-4866-8187-88aa874816d4-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:  Accepted
  Aggregate, F   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * stage:  Unreviewed => Accepted


Comment:

 Another instance of a crash due to mishandling of
 `get_source_expressions()` being `Expression | None`.

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bde7a1a79-bfb482f5-daa2-4b88-9963-6e1ed0456a24-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs does not work properly with models.Window and aggregation (was: Getting refs is not work properly with models.Window and aggregation)

[Django]

#34975: Getting refs does not work properly with models.Window and aggregation
-+-
 Reporter:  Sergey Nesterenko|Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:
  Aggregate, F   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bde61bc45-d9c4c0bc-b600-4189-b21b-50229dedd801-00%40eu-central-1.amazonses.com.

Re: [Django] #34975: Getting refs is not work properly with models.Window and aggregation

[Django]

#34975: Getting refs is not work properly with models.Window and aggregation
-+-
 Reporter:  Zwergpro |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  QuerySet, Window,| Triage Stage:
  Aggregate, F   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Zwergpro:

Old description:

> I found out that getting refs in QuerySet doesn't work with models.Window
> when I try to use aggregate method. It happened when I tried to update
> Django to 4.2.7.
>
> For example:
> {{{
> MyModel.objects.annotate(
> new_ordering=Window(RowNumber(),  order_by=[F('ordering')])
> ).aggregate(
> wrong_count=Count('id', filter=Q(ordering=F('new_ordering')))
> )
> }}}
>
> And as a result, it fails with:
> {{{
> Traceback (most recent call last):
>   File "/main.py", line 38, in 
> result = ordering_query.aggregate(
>   File "/python3.10/site-packages/django/db/models/query.py", line 592,
> in aggregate
> return self.query.chain().get_aggregation(self.db, kwargs)
>   File "/python3.10/site-packages/django/db/models/sql/query.py", line
> 398, in get_aggregation
> aggregate = aggregate_expr.resolve_expression(
>   File "/python3.10/site-packages/django/db/models/aggregates.py", line
> 70, in resolve_expression
> for ref in c.get_refs():
>   File "/python3.10/site-packages/django/db/models/expressions.py", line
> 418, in get_refs
> refs |= expr.get_refs()
>   File "/python3.10/site-packages/django/db/models/sql/where.py", line
> 233, in get_refs
> refs |= child.get_refs()
>   File "/python3.10/site-packages/django/db/models/expressions.py", line
> 418, in get_refs
> refs |= expr.get_refs()
>   File "/python3.10/site-packages/django/db/models/expressions.py", line
> 418, in get_refs
> refs |= expr.get_refs()
> AttributeError: 'NoneType' object has no attribute 'get_refs'
> }}}
>
> It happens because of Window.get_source_expressions return:
> {{{
> def get_source_expressions(self):
> return [self.source_expression, self.partition_by, self.order_by,
> self.frame]
> }}}
> and if we don't specify 'frame' (for example) it will be None in the
> list.
>
> And in models.aggregates.Aggregate.resolve_expression when we try to find
> all refs
> https://github.com/django/django/blob/47f9b8dca16b1fdc054b338d81eb080eabad0edf/django/db/models/aggregates.py#L70C30-L70C30
> it fails because of we have None in Window.get_source_expressions
> {{{
> def get_refs(self):
> refs = set()
> for expr in self.get_source_expressions():
> refs |= expr.get_refs()
> return refs
> }}}

New description:

 I found out that getting refs in QuerySet doesn't work with models.Window
 when I try to use aggregate method. It happened when I tried to update
 Django to 4.2.7.

 For example:
 {{{
 MyModel.objects.annotate(
 new_ordering=Window(RowNumber(),  order_by=[F('ordering')])
 ).aggregate(
 new_count=Count('id', filter=Q(ordering=F('new_ordering')))
 )
 }}}

 And as a result, it fails with:
 {{{
 Traceback (most recent call last):
   File "/main.py", line 38, in 
 result = ordering_query.aggregate(
   File "/python3.10/site-packages/django/db/models/query.py", line 592, in
 aggregate
 return self.query.chain().get_aggregation(self.db, kwargs)
   File "/python3.10/site-packages/django/db/models/sql/query.py", line
 398, in get_aggregation
 aggregate = aggregate_expr.resolve_expression(
   File "/python3.10/site-packages/django/db/models/aggregates.py", line
 70, in resolve_expression
 for ref in c.get_refs():
   File "/python3.10/site-packages/django/db/models/expressions.py", line
 418, in get_refs
 refs |= expr.get_refs()
   File "/python3.10/site-packages/django/db/models/sql/where.py", line
 233, in get_refs
 refs |= child.get_refs()
   File "/python3.10/site-packages/django/db/models/expressions.py", line
 418, in get_refs
 refs |= expr.get_refs()
   File "/python3.10/site-packages/django/db/models/expressions.py", line
 418, in get_refs
 refs |= expr.get_refs()
 AttributeError: 'NoneType' object has no attribute 'get_refs'
 }}}

 It happens because of Window.get_source_expressions return:
 {{{
 def get_source_expressions(self):
 return [self.source_expression, self.partition_by, self.order_by,
 self.frame]
 }}}
 and if we don't specify 'frame' (for example) it will be None in the list.

 And 

[Django] #34975: Getting refs is not work properly with models.Window and aggregation

[Django]

#34975: Getting refs is not work properly with models.Window and aggregation
-+-
   Reporter:  Zwergpro   |  Owner:  nobody
   Type:  Bug| Status:  new
  Component:  Database   |Version:  4.2
  layer (models, ORM)|   Keywords:  QuerySet, Window,
   Severity:  Normal |  Aggregate, F
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 I found out that getting refs in QuerySet doesn't work with models.Window
 when I try to use aggregate method. It happened when I tried to update
 Django to 4.2.7.

 For example:
 {{{
 MyModel.objects.annotate(
 new_ordering=Window(RowNumber(),  order_by=[F('ordering')])
 ).aggregate(
 wrong_count=Count('id', filter=Q(ordering=F('new_ordering')))
 )
 }}}

 And as a result, it fails with:
 {{{
 Traceback (most recent call last):
   File "/main.py", line 38, in 
 result = ordering_query.aggregate(
   File "/python3.10/site-packages/django/db/models/query.py", line 592, in
 aggregate
 return self.query.chain().get_aggregation(self.db, kwargs)
   File "/python3.10/site-packages/django/db/models/sql/query.py", line
 398, in get_aggregation
 aggregate = aggregate_expr.resolve_expression(
   File "/python3.10/site-packages/django/db/models/aggregates.py", line
 70, in resolve_expression
 for ref in c.get_refs():
   File "/python3.10/site-packages/django/db/models/expressions.py", line
 418, in get_refs
 refs |= expr.get_refs()
   File "/python3.10/site-packages/django/db/models/sql/where.py", line
 233, in get_refs
 refs |= child.get_refs()
   File "/python3.10/site-packages/django/db/models/expressions.py", line
 418, in get_refs
 refs |= expr.get_refs()
   File "/python3.10/site-packages/django/db/models/expressions.py", line
 418, in get_refs
 refs |= expr.get_refs()
 AttributeError: 'NoneType' object has no attribute 'get_refs'
 }}}

 It happens because of Window.get_source_expressions return:
 {{{
 def get_source_expressions(self):
 return [self.source_expression, self.partition_by, self.order_by,
 self.frame]
 }}}
 and if we don't specify 'frame' (for example) it will be None in the list.

 And in models.aggregates.Aggregate.resolve_expression when we try to find
 all refs
 
https://github.com/django/django/blob/47f9b8dca16b1fdc054b338d81eb080eabad0edf/django/db/models/aggregates.py#L70C30-L70C30
 it fails because of we have None in Window.get_source_expressions
 {{{
 def get_refs(self):
 refs = set()
 for expr in self.get_source_expressions():
 refs |= expr.get_refs()
 return refs
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018bdde1938f-56612631-fa3d-43e2-b63d-ebfd5f9123dd-00%40eu-central-1.amazonses.com.