Should we change the default for the distinct argument to the count and has pair functions?

[j4mie]

The distinct argument to any Aggregate subclass (Count, Sum, Avg etc) in the ORM defaults to False:

>>> print(models.Widget.objects.only("id").annotate(name_count=Count("name")).query)
SELECT "widget"."id", COUNT("widget"."name") AS "name_count" FROM "widget" GROUP BY "widget"."id"

However, in our pairs.count and pairs.has shortcuts, we override the default to be True:

>>> prepare, project = pairs.count("name")
>>> print(prepare(models.Widget.objects.all()).query)
SELECT "widget"."id", COUNT(DISTINCT "widget"."name") AS "name_count" FROM "widget" GROUP BY "widget"."id"

This was done because the DISTINCT version is arguably less surprising and gives the correct answer more of the time, ie it usually expresses what the developer intended, especially when multiple aggregations are combined.

However, to a developer who already knows how these aggregates work in Django, it's probably much more surprising that the defaults are reversed in django-readers.

So: should we change the default here and bring django-readers back into alignment with Django itself? This would also simplify the implementation of pairs.count etc (see #56). However, it would be a backwards-incompatible change and would therefore require a major version bump 😬

[carltongibson]

Hi @j4mie -- I've looked at this several times...

I'd say it's marginal but over the life of the project it's probably better to be consistent with the ORM behaviour, that being "expected".

[j4mie]

Thanks @carltongibson. I was leaning towards agreeing with this anyway, and following your comment and a discussion interally we've decided to go ahead. The backwards-incompatible change has been made in #56

[carltongibson]

Super. Glad piping up at least helped with a decision 😅

Keep up the good work!

[j4mie]

Decision: this has now been done in #56 and will be released in version 2.0