#34564: returning None instead of zero in Count annotation
-------------------------------------+-------------------------------------
Reporter: Amin Aminian | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: count, orm, | Triage Stage: Accepted
annotate |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Amin Aminian):
Replying to [comment:2 Simon Charette]:
> I think this should be considered a release blocker as this is an
unintended regression that will affect quite a few users given how common
count annotations are over multiple valued relationships.
>
> Unlike `ArrayAgg` and friends in
fee87345967b3d917b618533585076cbfa43451b there was not deprecation period
to force the usage of `default=0` for `Count` in
9f3cce172f6913c5ac74272fa5fc07f847b4e112.
>
> A simple solution for a backport would be to adjust `Count.__init__` to
do `extra.setdefault("default", 0)` (or define in it's signature for
enhanced introspection) so `Coalesce` is used.
>
> Happy to submit a patch if you agree.
>
> Amin, you can use `Count(..., default=0)` in the mean time.
I have read 9f3cce172f6913c5ac74272fa5fc07f847b4e112. The thing that i'm
not understanding is why we want to return None instead of zero. What is
the reason ? How this is benefiting us ?
As far as I have read the changes, in Django==4.2, we have an attribute
called `empty_result_set_value` which is equal to `NotImplemented` in
`BaseExpression`:
{{{
class BaseExpression:
empty_result_set_value = NotImplemented
...
}}}
And aggregate classes have implemented this. All of classes have
`empty_result_set_value = None` instead of `Count` class and `RegrCount`
class, which it is `empty_result_set_value = 0` in those classes. And
because of this attr, we can't use `default=0`:
{{{
class Aggregate(Func):
...
empty_result_set_value = None
def __init__(
self, *expressions, distinct=False, filter=None, default=None,
**extra
):
...
if default is not None and self.empty_result_set_value is not
None:
raise TypeError(f"{self.__class__.__name__} does not allow
default.")
...
}}}
So as far as I understand, we are considering `empty_result_set_value` as
kind of a default value. So why don't we just return
`empty_result_set_value` in case of being None in `convert_value` property
?
--
Ticket URL: <https://code.djangoproject.com/ticket/34564#comment:5>
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 view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018819a54a64-40b7cd91-cd38-4511-88fd-b8ccb230cd28-000000%40eu-central-1.amazonses.com.
