Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  aggregation  | Triage Stage:  Ready for
  coalesce in queryset   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by GitHub ):

 In [changeset:"0f3e1a54bfa19f034f88bf3c25c67402d19e906c" 0f3e1a54]:
 {{{
 #!CommitTicketReference repository=""
 revision="0f3e1a54bfa19f034f88bf3c25c67402d19e906c"
 Refs #26430 -- Removed unused branch in sql.Query.get_count().

 Now that sql.Query.get_aggregation() properly deals with empty result
 sets summary Count() annotations cannot result in None.

 Unused since 9f3cce172f6913c5ac74272fa5fc07f847b4e112.
 }}}

-- 
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/068.e02f22ea5dbc7f7111afcb800cb59209%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  aggregation  | Triage Stage:  Ready for
  coalesce in queryset   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"9f3cce172f6913c5ac74272fa5fc07f847b4e112" 9f3cce17]:
 {{{
 #!CommitTicketReference repository=""
 revision="9f3cce172f6913c5ac74272fa5fc07f847b4e112"
 Refs #26430 -- Re-introduced empty aggregation optimization.

 The introduction of the Expression.empty_aggregate_value interface
 allows the compilation stage to enable the EmptyResultSet optimization
 if all the aggregates expressions implement it.

 This also removes unnecessary RegrCount/Count.convert_value() methods.
 Disabling the empty result set aggregation optimization when it wasn't
 appropriate prevented None returned for a Count aggregation value.

 Thanks Nick Pope for the review.
 }}}

-- 
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/068.3c8cc30b50c9d105493cbbd019939832%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  aggregation  | Triage Stage:  Ready for
  coalesce in queryset   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

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


Comment:

 In [changeset:"f3112fde981052801e0c2121027424496c03efdf" f3112fde]:
 {{{
 #!CommitTicketReference repository=""
 revision="f3112fde981052801e0c2121027424496c03efdf"
 Fixed #26430 -- Fixed coalesced aggregation of empty result sets.

 Disable the EmptyResultSet optimization when performing aggregation as
 it might interfere with coalescence.
 }}}

-- 
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/068.f6f1bb2e77cb049fb2b9e5858ebc344b%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Ready for
  coalesce in queryset   |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * stage:  Accepted => Ready for checkin


-- 
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/068.e20c7540493d47f4fa419538979f40b0%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * needs_better_patch:  1 => 0
 * needs_docs:  1 => 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/068.da56e54c708f0f22f3df96b3591662c1%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  1|  Needs documentation:  1
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by GitHub ):

 In [changeset:"5371ad1d149480bf64c37b3273e06a1c76fe4b90" 5371ad1d]:
 {{{
 #!CommitTicketReference repository=""
 revision="5371ad1d149480bf64c37b3273e06a1c76fe4b90"
 Refs #26430 -- Added tests for PostgreSQL-specific aggregates on
 EmptyQuerySets and used subTest().
 }}}

-- 
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/068.479252c25b7fedb76170803cbd198955%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  Simon
 |  Charette
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  1|  Needs documentation:  1
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * owner:  nobody => Simon Charette
 * needs_better_patch:  0 => 1
 * status:  new => assigned
 * needs_docs:  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/068.1a0cb2291dc03905b1e034d82c2edd60%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * cc: Nick Pope (added)


-- 
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/068.8428dbfcd3c75589991d910977bc7023%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
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


Comment:

 Tried implementing the above in
 https://github.com/django/django/pull/14430

-- 
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/068.1c7fb39f5c5ff2fad9dc277973c9f4aa%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 Sorry for the delayed answer Nick.

 > It seems that ​https://github.com/django/django/pull/6361 would be the
 way forward. Was there a reason you abandoned that approach, Simon? Would
 you be happy for me to pick that up again?

 I think it would be the most straightforward way to get there but the main
 issue with the `get_empty_result` approach is coalescence to database
 expressions e.g. `Coalesce('col', Now())`.

 It's tempting to mock our way there and ''cheat'' for provided expressions
 in implementing Python equivalent (e.g. `Now.get_empty_result ->
 timezone.now`) but it has its limits (e.g. imagine coalescing to calling a
 database procedure). In the end `EmptyResultSet` is meant to be an
 optimization so I believe we should turn it off in circumstances where it
 affects the correctness of the returned results.

 A simpler approach, inspired by Anssi's comment:4, would be to add a
 `SQLCompiler.supports_empty_result_set = False` property and set it to
 `False` for `SQLAggregateCompiler`. Parts of the code that raise this
 exception could then be adjusted not to do so when the current compiler
 doesn't support it.

 (completely untested)

 {{{#!diff
 diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py
 index 8d3648b393..0b02287d2a 100644
 --- a/django/db/models/lookups.py
 +++ b/django/db/models/lookups.py
 @@ -392,7 +392,7 @@ def process_rhs(self, compiler, connection):
  except TypeError:  # Unhashable items in self.rhs
  rhs = [r for r in self.rhs if r is not None]

 -if not rhs:
 +if not rhs and compiler.supports_empty_result_set:
  raise EmptyResultSet

  # rhs should be an iterable; use batch_process_rhs() to
 diff --git a/django/db/models/sql/compiler.py
 b/django/db/models/sql/compiler.py
 index 7264929da8..6cfcbf7f9f 100644
 --- a/django/db/models/sql/compiler.py
 +++ b/django/db/models/sql/compiler.py
 @@ -25,6 +25,7 @@ class SQLCompiler:
  r'^(.*)\s(?:ASC|DESC).*',
  re.MULTILINE | re.DOTALL,
  )
 +supports_empty_result_set = True

  def __init__(self, query, connection, using):
  self.query = query
 @@ -1636,6 +1637,11 @@ def pre_sql_setup(self):


  class SQLAggregateCompiler(SQLCompiler):
 +# Due to the nature of aggregation coalescence some expressions might
 +# need to be interpreted on the database to return the correct value
 +# when dealing with an empty result set.
 +supports_empty_result_set = False
 +
  def as_sql(self):
  """
  Create the SQL for this query. Return the SQL string and list of

 }}}

 I guess we could also use an hybrid approach where `SQLAggregateCompiler`
 only sets `supports_empty_result_set = False` if any of
 `outer_query.annotation_select.items()` is not a literal value prior to
 proceeding with the compilation/`as_sql` phase. That would keep the
 optimization for most cases but fallback to the database when it cannot be
 computed on the Python side.

-- 
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/068.56f1ea2c92337b5bb3f6cfd58c93ef4e%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Anton Agestam):

 * cc: Anton Agestam (added)


-- 
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/068.926721f96b6e748fca212915bdcd4d62%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Hannes Ljungberg):

 * cc: Hannes Ljungberg (added)


-- 
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/068.b55034edb956bb2b79bea622a8fdf771%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * cc: Simon Charette (added)


-- 
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/068.729bb7afe61acc1b1fd5a99e2ba022a9%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Nick Pope):

 Replying to [comment:7 Simon Charette]:
 > This happens to be addressed by
 https://github.com/django/django/pull/12602

 I was looking into reviving https://github.com/django/django/pull/12602.

 It seems that some parts of that were already addressed in
 c2d4926702045e342a668057f0a758eec9db9436.
 Notably this
 
[https://github.com/django/django/commit/c2d4926702045e342a668057f0a758eec9db9436
 #diff-68490047146b30a432d0d7b7793005e89c112289dcb273fa082b4e9df9b81af7R977
 solved] the `None` instead of `0` issue for `Count()` in
 `aggregation_regress.tests.AggregationTests.test_empty_filter_aggregate`.

 It doesn't seem to be true that this ticket will be addressed by that PR
 as implied by
 [https://github.com/django/django/pull/11715#issuecomment-602050401 this
 comment].
 Changing the test to use `Coalesce()` as below still fails with
 `total_age` being `None`. There is nothing to determine that the second
 argument of `Coalesce()` should be used.

 {{{#!diff
  self.assertEqual(
 -
 
Author.objects.filter(id__in=[]).annotate(Count('friends')).aggregate(Count('pk')),
 -{'pk__count': 0},
 +
 
Author.objects.filter(pk__in=[]).annotate(Count('friends')).aggregate(total_age=Coalesce(Sum('age'),
 0)),
 +{'total_age': 0},
  )
 }}}

 In the case of `Count()` we get 0 because of `Count.convert_value()`
 explicitly returning `0` if the value is `None`.

 It seems that https://github.com/django/django/pull/6361 would be the way
 forward.
 Was there a reason you abandoned that approach, Simon? Would you be happy
 for me to pick that up again?

-- 
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/068.84ce98e6dca56f0ac91b6658d3ae12d3%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  Ryan Prater  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 This happens to be addressed by
 https://github.com/django/django/pull/11715

-- 
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/068.2e8567624fb4f16473e280db633d6fe9%40djangoproject.com.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  ryanprater   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:  Accepted
  coalesce in queryset   |
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by akaariai):

 * stage:  Unreviewed => Accepted


Comment:

 See #26433 for related issue and a new idea for solving this.

 Marking as accepted, I think we can solve this without unreasonable
 effort.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.78a6619c1db4207012f79ef4654d162b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  ryanprater   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:
  coalesce in queryset   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by charettes):

 FWIW I gave a naive try at implementing the `get_empty_result()` approach
 suggested by Josh:

 https://github.com/django/django/pull/6361

 The full suite pass but coalesced results relying on database functions
 (e.g. `NOW()`) are not supported.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.386060d3618b7a4641abeea8fd02280e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  ryanprater   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:
  coalesce in queryset   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by akaariai):

 Maybe we could somehow skip throwing EmptyResultSet when doing an
 .aggregate() call, but throw it when doing normal queries. One idea is to
 use query.add_context('in_aggregate_query', True), and then check
 compiler.query.context before throwing the EmptyResultSet.

 Another possible idea is to add as_python() methods to pretty much every
 part of the ORM. The as_python() method would run the expressions as
 Python code, so you could run whole querysets without actually touching
 the database. For the case where the query for .aggregate() generates an
 EmptyResultSet we would run the queryset in Python against one row having
 all None values, thus producing the wanted answer. This could be extremely
 convenient for testing, too. OTOH doing this properly will require at
 least a couple of months of work, so this is for sure overkill for this
 issue.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.577fae7402faf5c9d6bf177fcf3afbb9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  ryanprater   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:
  coalesce in queryset   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by charettes):

 I agree with Josh that the `on_empty_result` approach seems like a lot of
 work for very little gain but I can't see any other way to fix this issue
 appropriately.

 @ryanprater not throwing `EmptyResultSet` in the `In` lookup would solve
 your particular issue but the underlying one would remain, using
 `Coalesce` with aggregation can result in an unexpected return value as
 `EmptyResultSet` is thrown by other part of Django (`queryset.none()`
 relies on it).

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.c978e700d65e4f280ec11067d18c2a2f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  ryanprater   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:
  coalesce in queryset   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by ryanprater):

 I can agree that detecting specific expressions is overkill and for that
 matter, not scalable. However, I don't think an "empty list in a filter
 clause seems like broken behavior". As you mentioned about valid use
 cases: I'm using this with a SelectMultiple which obviously allows users
 to select no options. Writing custom validation to prevent an empty query
 should be the responsibility of the developer, but not enforced at the
 framework level.

 It seems like this could be avoided by modifying the logic of `In.rhs`
 (and/or not throwing the `EmptyResultSet` at all?), but this is my first
 bug and I'm not versed enough to discern.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.8ce1916cfaed5eed168e02c60c048e2a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #26430: Coalesce in Aggregations ignored when EmptyResultSet returned

[Django]

#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-+-
 Reporter:  ryanprater   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.9
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  aggregation  | Triage Stage:
  coalesce in queryset   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by jarshwah):

 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 I do agree that this is a bug, but I don't think the correct fix is to
 detect specific expressions and apply custom logic for each one. Simon
 mentioned nested expressions which would be very difficult to resolve the
 correct behaviour for.

 For what it's worth, a `Count` will also show `None` in an EmptyResultSet
 even though its converters should coerce `None` to `0`. The reason it
 doesn't is because the expressions and aggregates aren't actually used.
 Everything is Noned.

 If I was writing this from scratch I'd raise an error on receiving an
 EmptyResultSet. An empty list in a filter clause seems like broken
 behaviour. We can't do that though, because it's backwards incompatible,
 and there's probably valid use cases for this behaviour now.

 A possible fix would be to implement something like `on_empty_result` for
 Expressions, to let each subclass decide what it should return if it's not
 executed. It would have to recursively call this method, and then decide
 which children should be allowed to propagate their values. But honestly,
 this seems like a lot of work and complexity for very little gain.

 Short of documentation changes or a deprecation, I'd be tempted to wontfix
 this. I'm certainly open to ideas I haven't considered though.

--
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.531db085740a40f3f01c91d69bcde4a0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.