Opened 16 years ago

Closed 16 years ago

#7528 closed (duplicate)

Foreign keys in search_fields give wrong results some hits duplicated, some missing)

Reported by: Paul Winkler Owned by: nobody
Component: contrib.admin Version: dev
Severity: Keywords: nfa-fixed
Cc: Triage Stage: Fixed on a branch
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In the docs at http://www.djangoproject.com/documentation/0.96/model-api/
about search_fields, it says you can use 'the lookup API “follow” notation' to get a foreign key into your search_fields.
Like this:

class FilmClip(models.Model):
    ...
    class Admin:
        ...
        search_fields = (...
                         'filmcliplog__description'   # This is the
line that causes problems
                         )

class FilmClipLog(models.Model):
    clip = models.ForeignKey(FilmClip,
                             edit_inline=models.TABULAR,
                             num_in_admin=3)
    description = ...

There are two problems with the resulting behavior:

1) Any matching FilmClip that has at least one FilmClipLog will show
up in the results twice.

2) Any matching FilmClip that has no FilmClipLogs will not show up at
all.

From the mysql query log, I've found that the query looks like:
{{{
  SELECT ... FROM `filmy_filmclip` INNER JOIN `filmy_filmcliplog` ...
}}}

I think the correct query would be:

{{{
  SELECT DISTINCT ... FROM `filmy_filmclip` LEFT OUTER JOIN
`filmy_filmcliplog` ...
}}}

Originally discussed at http://groups.google.com/group/django-users/browse_thread/thread/da44ae1542436d18
(no replies)

Change History (3)

comment:1 by Paul Winkler, 16 years ago

Version: 0.96SVN

I have checked the behavior on current trunk (as of r7732).

The query now uses LEFT OUTER JOIN, which I've confirmed fixes problem 2.

But it doesn't use DISTINCT, so I still see duplicates (problem 1).

Unfortunately I still don't know where these queries are generated so I can't send you a patch until I understand Django and the admin app a lot better.
Meanwhile I hope somebody else knows how to fix it quickly :)

comment:2 by Paul Winkler, 16 years ago

I *almost* found a workaround:

class ClipManager(models.Manager):
    def get_query_set(self):
        qs = super(ClipManager, self).get_query_set()
        return qs.distinct()

class FilmClip(models.Model):
    ...
    class Admin:
        ...
        manager = ClipManager()

Unfortunately, against django trunk this blows up like so:

Request Method: GET
Request URL: http://localhost:8000/admin/filmy/filmclip/
Django Version: 0.97-pre-SVN-unknown
Python Version: 2.4.4
Installed Applications:
['django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'brainpower.filmy']
Installed Middleware:
('django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.middleware.doc.XViewMiddleware')


Traceback:
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  82.                 response = callback(request, *callback_args, **callback_kwargs)
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  62.             return view_func(request, *args, **kwargs)
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in change_list
  757.         cl = ChangeList(request, model)
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in __init__
  581.         self.query_set = self.get_query_set()
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in get_query_set
  740.                 qs = qs & other_qs
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/db/models/query.py" in __and__
  222.         combined.query.combine(other.query, sql.AND)
File "/home/pw/builds/brainpower/builds/20080624/brainpower/lib/python2.4/site-packages/django/db/models/sql/query.py" in combine
  309.         assert self.distinct == rhs.distinct, \

Exception Type: AssertionError at /admin/filmy/filmclip/
Exception Value: Cannot combine a unique query with a non-unique query.

I can fix this with the following patch:

--- django/contrib/admin/views/main.py	(revision 7739)
+++ django/contrib/admin/views/main.py	(working copy)
@@ -737,6 +737,8 @@
                 other_qs = QuerySet(self.model)
                 other_qs.dup_select_related(qs)
                 other_qs = other_qs.filter(reduce(operator.or_, or_queries))
+                # I don't really understand this hack to allow working around #7528
+                other_qs.query.distinct = qs.query.distinct
                 qs = qs & other_qs
 
         if self.opts.one_to_one_field:

And then everything seems to finally work as I expected.
Unfortunately I don't know if this patch has other implications.
And all it does is allow a workaround; it still doesn't fix the original problem.
IMHO you shouldn't have to create a custom manager; DISTINCT should be the default behavior.

I hope somebody is reading this :)

comment:3 by Karen Tracey <kmtracey@…>, 16 years ago

Keywords: nfa-fixed added
Resolution: duplicate
Status: newclosed
Triage Stage: UnreviewedFixed on a branch

The remaining problem (missing DISTINCT) has been fixed as part of the newforms-admin work, see ticket #2818.

To get the fix you can either switch from trunk to newforms-admin branch (I know, I already made you switch to trunk from 0.96 to get the first part fixed...you've happened on a problem that is only completely fixed in the latest latest), or look at the changeset for #2818 and apply a similar fix to your copy of trunk, or wait for newforms-admin to be merged back to trunk (hopefully in weeks, not months). Note switching to newforms-admin is another backwards-incompatible change, since the way of specifying admin stuff is different, see:

http://code.djangoproject.com/wiki/NewformsAdminBranch

for details.

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