Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#3732 closed (duplicate)

inconsistent values('col1', 'col2').distinct().count()

Reported by: David S. <davidschein@…> Owned by: adrian
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qs-rf
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

A ValueQuerySet with selected columns and using distinct returns a count that is accurate only if _result_cache exists--because it can get its len().
If _result_cache does not exists then the SQL to fetch to count is inaccurate because it relies on the PK rather than the selected columns.

For instance:

>>> cs = Course.objects.filter(subject__code="ACC", course_num=207).values('subject', 'course_num')
>>> cs
[{'course_num': '207', 'subject': 'ACC'}, {'course_num': '207', 'subject': 'ACC'}]
>>> cs.count()
2
>>> cs.distinct().count()
2L
>>> csdistinct = cs.distinct()
>>> csdistinct.count()
2L
>>> csdistinct
[{'course_num': '207', 'subject': 'ACC'}]
>>> csdistinct.count()
1

The attached patch has been tested and works on Postgres and Oracle and, though the documentation explicitly says it should work, it seems to fail with SQLite (at least testing in its shell.)

Attachments (1)

count_distinct.patch (1.2 KB) - added by David S. <davidschein@…> 8 years ago.
patch

Download all attachments as: .zip

Change History (4)

Changed 8 years ago by David S. <davidschein@…>

patch

comment:1 Changed 8 years ago by Simon G. <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 7 years ago by SmileyChris

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #2939. My patch there is pretty much the same as this (but abstracted) and includes tests

comment:3 Changed 7 years ago by mtredinnick

  • Keywords qs-rf added
Note: See TracTickets for help on using tickets.
Back to Top