Django

Code

Ticket #1535 (closed: fixed)

Opened 3 years ago

Last modified 2 years ago

[patch] ManyToMany query problems on magic-removal

Reported by: Russell Cloran <russell@hbd.com> Assigned to: adrian
Milestone: Component: Database layer (models, ORM)
Version: magic-removal Keywords:
Cc: mir@noris.de Triage Stage: Unreviewed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

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),

Attachments

query.diff (448 bytes) - added by mir@noris.de on 04/03/06 22:53:11.
Russel's patch from the ticket description
mysqllogs.txt (0.7 kB) - added by Russell Cloran <russell@hbd.com> on 04/05/06 05:02:18.
(broken) SQL queries generated
pythonconsole.txt (0.9 kB) - added by Russell Cloran <russell@hbd.com> on 04/05/06 05:03:37.
My test session output
models.py (307 bytes) - added by Russell Cloran <russell@hbd.com> on 04/05/06 05:04:49.
Entire test models.py
models.2.py (1.4 kB) - added by lukeplant on 04/05/06 06:07:10.
models.py - can be inserted into a new folder in modeltests to run tests.

Change History

04/03/06 22:48:54 changed by mir@noris.de

  • cc set to mir@noris.de.

04/03/06 22:53:11 changed by mir@noris.de

  • attachment query.diff added.

Russel's patch from the ticket description

04/03/06 22:57:15 changed by mir@noris.de

  • summary changed from ManyToMany query problems on magic-removal to [patch] ManyToMany query problems on magic-removal.

- 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.

04/04/06 14:28:08 changed by lukeplant

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?

04/05/06 04:24:09 changed by Russell Cloran <russell@hbd.com>

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.

04/05/06 05:02:18 changed by Russell Cloran <russell@hbd.com>

  • attachment mysqllogs.txt added.

(broken) SQL queries generated

04/05/06 05:03:37 changed by Russell Cloran <russell@hbd.com>

  • attachment pythonconsole.txt added.

My test session output

04/05/06 05:04:49 changed by Russell Cloran <russell@hbd.com>

  • attachment models.py added.

Entire test models.py

04/05/06 06:07:10 changed by lukeplant

  • attachment models.2.py added.

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

04/05/06 06:58:23 changed by lukeplant

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!

04/05/06 07:00:39 changed by lukeplant

  • status changed from new to closed.
  • resolution set to fixed.

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

04/05/06 07:04:34 changed by lukeplant

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

06/04/06 11:56:43 changed by akaihola

Is this related to #2080?


Add/Change #1535 ([patch] ManyToMany query problems on magic-removal)




Change Properties
Action