Opened 18 years ago

Closed 18 years ago

Last modified 18 years ago

#1530 closed enhancement (fixed)

[patch] making distinct=True work with get_count

Reported by: anonymous Owned by: Adrian Holovaty
Component: Core (Other) Version: 0.91
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently get_count() ignores a distinct=True argument. This patch fixes that.

Unfortunately, company firewall issues don't allow me to use svn, so I don't have an svn-diff patch. Sorry about pasting code into the ticket body. For the same reason, I'm using the 0.91 tarball, not the svn version, though I believe the patch is the same for both (I don't think the method in question has changed). In magic-removal, the same idea should work, but it looks like the syntax would be slightly different.

Here's the changed method in django/core/meta/__init__.py (commented out lines are the originals, with the changed version below them):

def function_get_count(opts, **kwargs):
    kwargs['order_by'] = []
    kwargs['offset'] = None
    kwargs['limit'] = None
    kwargs['select_related'] = False
#    _, sql, params = function_get_sql_clause(opts, **kwargs)
    select, sql, params = function_get_sql_clause(opts, **kwargs)
    cursor = db.db.cursor()
#    cursor.execute("SELECT COUNT(*)" + sql, params)
    cursor.execute("SELECT COUNT(" + (kwargs.get('distinct') and "DISTINCT %s" % select[0] or "*")  + ")" + sql, params)
    return cursor.fetchone()[0]

Change History (4)

comment:1 by Adam Endicott <leftwing17@…>, 18 years ago

Oops, sorry, didn't mean to submit as anonymous.

comment:2 by Adrian Holovaty, 18 years ago

I was looking over this patch and had a "wait a minute" thought...

What's the use-case for this? In which case would get_count(distinct=True) return a different result than get_count()?

The distinct option uses SELECT DISTINCT across every row, and each row is guaranteed to be distinct because it requires a primary key.

comment:3 by Adam Endicott <leftwing17@…>, 18 years ago

Yeah, see, this is why I asked on django-dev first ;). Its possible I'm missing something, but here's my use case:

class Category(meta.Model):
    name = meta.CharField(maxlength=100)

class Business(meta.Model):
    name = meta.CharField(maxlength=100)
    category = meta.ManyToManyField(Category)
>>> c1 = categories.Category(name='pizzarias')
>>> c2 = categories.Category(name='pizza restaurants')
>>> cpk = businesses.Business(name="Bob's pizza")
>>> cpk.set_category([c1.id, c2.id])
>>> businesses.get_count(category__name__icontains='pizza')
2
>>> businesses.get_list(category__name__icontains='pizza')
["Bob's pizza", "Bob's pizza"]
>>> businesses.get_count(category__name__icontains='pizza', distinct=True) # with this patch
1
>>> businesses.get_list(category__name__icontains='pizza', distinct=True)
["Bob's pizza"]

(sorry about any errors introduced by simplifying this)

Basically it looks like the generated SQL is causing a hit for each category that matches the criteria. Now that I think about it though, it may just be a coincidence that this patch is working for my case, because its distinguishing on the business.id, which just happens to be what select[0] gives in this case? I'm not enough of an SQL guru for this, I just know this works for my particular use.

comment:4 by Malcolm Tredinnick, 18 years ago

Resolution: fixed
Status: newclosed

(In [2902]) Fixed #1530 -- make count() respect distinct() on QuerySets. Create some
tests for this as well. Thanks to Adam Endicott for the original patch on which
this is based.

Note: See TracTickets for help on using tickets.
Back to Top