[patch] ManyToMany query problems on magic-removal

[Russell Cloran <russell@…>]

Hi,

When querying a model where one of the query terms includes a ManyToMany relation, some data which should appear in the result doesn't. This occurs if the ManyToMany relation does not have any data. To be more clear, here is an example:

class Issue:
    ...
    cc = models.ManyToManyField(User, blank=True)
    client = models.ForeignKey(User)
    ....

>>> Issue.objects.filter(Q(client=u.id))
[Issue 1, Issue 7]
>>> Issue.objects.filter(Q(cc__id__exact=u.id))
[Issue 7, Issue 8]
>>> Issue.objects.filter(Q(cc__id__exact=u.id) | Q(client=u.id))
[Issue 7, Issue 8]

The result for the last query should include "Issue 1", as far as I can tell. The following patch solves the problem, but it was derived by watching which queries happened and hacking the code to make my case work, so I don't know what it breaks :)

===================================================================
--- django/db/models/query.py   (revision 2546)
+++ django/db/models/query.py   (working copy)
@@ -769,7 +769,7 @@
     if intermediate_table:
         joins[backend.quote_name(current_table)] = (
             backend.quote_name(intermediate_table),
-            "INNER JOIN",
+            "LEFT OUTER JOIN",
             "%s.%s = %s.%s" % \
                 (backend.quote_name(table),
                 backend.quote_name(current_opts.pk.column),

[mir@…]

Russel's patch from the ticket description

[mir@…]

• there's a patch for the problem in the ticket. Note that I haven't checked or verified the problem, I merely cleaned the ticket up.

[Luke Plant]

Strange - I have an equivalent model and it works fine (except I have to add .distinct() to remove the duplicated related item). Could you post a patch in the form of an addition to the test suite?

[Russell Cloran <russell@…>]

Luke - which database are you using? I'm *far* from a SQL guru, so I'm not sure if this will make a difference (afaict, it *shouldn't*). I am using MySQL. I will attach complete test output shortly.

[Russell Cloran <russell@…>]

(broken) SQL queries generated

[Russell Cloran <russell@…>]

My test session output

[Russell Cloran <russell@…>]

Entire test models.py

[Luke Plant]

models.py - can be inserted into a new folder in modeltests to run tests.

[Luke Plant]

OK, I've attached a version of models.py that can be slotted right into the tests. The tests do fail, on both postgres and mysql (I'm not sure what was different about my 'equivalent' model -- it obviously wasn't equivalent!). With your patch to query.py, all the tests pass.

I think the patch is good. Normally when you (implicitly) add a join in Django, you are also adding a WHERE clause that tests just the 'right hand side' of the join, so INNER JOIN and LEFT OUTER JOIN are identical. With the combination of a clause that tests the left hand side (containing the foreign key) and a clause that tests the m2m table on the right hand side, combined using OR, you have the edge case where they are different, and you need the LEFT JOIN. If this patch adds complications with more complicated joins and queries, I don't know what they are, and it's difficult to work it out without tests or more concrete examples.

I have added the tests to the test suite, and will commit the patch shortly. Thanks for catching this Russell!

[Luke Plant]

(In [2613]) magic-removal: Fixed #1535 (bug with combined m2m and m2o to the same model)

[Luke Plant]

Oops, sorry Russell, I meant to acknowledge the patch in my log message. I fixed it up afterwards for the record.

[Antti Kaihola]

Is this related to #2080?