Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#7047 closed (fixed)

filter() with Q spanning ManyToMany relations return wrong results

Reported by: Waldemar Kornewald Owned by: nobody
Component: Database layer (models, ORM) Version: queryset-refactor
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I'll try to simplify our situation, so the code samples have nothing to do with our actual models. Currently, we use sqlite with Python 2.5 and Django queryset-refactor (trunk has similar problems, though).

We have models similar to these:

class Tag(Model):
    name = CharField(max_length=100)
    hits = ManyToManyField(Article, related_name='tags')

class Category(Model):
    name = CharField(max_length=100)
    hits = ManyToManyField(Article, related_name='categories')

In our DB we have an Article object that has two tags ('taga' and 'tagb') and two categories ('cata' and 'catb').

Then we run the following query (find an Article that has both 'taga' and 'tagb' in the tags and/or categories):

Article.objects.filter(
    Q(categories__name__startswith='taga') \
    | \
    Q(tags__name__startswith='taga') \
    ,
    Q(categories__name__startswith='tagb') \
    | \
    Q(tags__name__startswith='tagb') \
    ).distinct()

The query will not find the article (although it should). Now, if we swap the attribute accessing order (i.e., replace Q(categories__...) | Q(tags__...) with Q(tags__...) | Q(categories__...)) it'll work correctly.

In other words: Only the first attribute (categories__...) will allow for matching multiple times whereas the second attribute (tags__...) only allows for matching once. Note, this actually seems to only depend on the attribute order of the *last* Q group (in the snippet it's the Q pair for 'tagb').

Other example queries that work correctly with the above code snippet:

  • 'cata' and 'catb'
  • 'taga' and 'cata'
  • 'cata' and 'taga' and 'catb'

Change History (5)

comment:1 by Malcolm Tredinnick, 16 years ago

Keywords: qs-rf removed
Triage Stage: UnreviewedAccepted
Version: SVNqueryset-refactor

Fixed version and keywords, since report is against the branch code.

comment:2 by Malcolm Tredinnick, 16 years ago

Whilst this example does demonstrate that Django isn't generating quite the correct SQL, your sample filter there is expected to return no results. Ignoring the category bits (which obviously won't match anything), it's asking for a tag that has a name that startswith 'taga' and also startswith 'tagb'. That is always the empty set.

If you're wanting -- and I suspect you are -- to filter on tags that start with 'taga' plus (other) tags that start with 'tagb', you need two filter calls. See the db-api.txt documentation, in the section labelled "Spanning multi-valued relationships" (only in queryset-refactor) for an explanation of the difference and why it's necessary.

Leaving the ticket open, since it shows a way to create the incorrect SQL, but just noting that you'll have to change your filter in any case (the correct way doesn't work properly in this example at the moment, either, so that's why it's a bug).

comment:3 by Waldemar Kornewald, 16 years ago

Then, I misunderstood the explanation in QuerysetRefactorBranch. I thought that filter().filter() would try to find tags that start with both strings at the same time. But I have the impression that even the filter(..., ...) version isn't really correct because, depending on the order, I am actually able to get the desired results. According to your explanation my query should *always* return an empty set, but it doesn't.

comment:4 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: newclosed

(In [7462]) queryset-refactor: Fixed some bugs in the multi-valued filtering behaviour
introduced in [7317]. It was failing in a couple of different ways on some
complex Q() combinations.

Fixed #7047

comment:5 by Waldemar Kornewald, 16 years ago

Yay! It works! Thank you *so* much!

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