Code

#18595 closed Bug (invalid)

Wrong table used in get_query_set when two Models have ManyToMany Fields to a common third Model using related_name='+'

Reported by: sean@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal 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

Summary: Wrong table used in get_query_set when two Models have ManyToMany Fields to a common third Model using related_name='+'

Description:
I discovered this bug after I applied a patch from a previous bug report: #15932

Using this patch:
#15932/15932-1.4rc1-Fixed-model-validation-when-using-hidden-related_name.patch

I have two models that each have an m2m field defiend as: admins = ManyToManyField( User, related_name='+' )
Accessing the first model's "admins" field results in a query being performed on the second model's underlying table.

models.py:

from django.db import models
from django.contrib.auth.models import User

class Company( models.Model ):
        name = models.CharField( max_length = 100 )
        admins = models.ManyToManyField( User, related_name = '+' )
        

class Location( models.Model ):
        company = models.ForeignKey( Company )
        address = models.CharField( max_length = 100 )
        admins = models.ManyToManyField( User, related_name = '+' )

manage.py shell

In [1]: from testapp.models import User, Company, Location

In [2]: from django.db import connection

In [3]: u = User.objects.get( pk=1 )

In [4]: c = Company( name='Test Co.')

In [5]: c.save()

In [6]: l = Location( company = c, address='123 Test St' )

In [7]: l.save()

In [8]: c.admins.add( u )

In [13]: c.admins.all()
Out[13]: []

In [14]: connection.queries
Out[14]: 
[{'sql': "INSERT INTO `testapp_company` (`name`) VALUES ('Test Co.')",
  'time': '0.000'},
 {'sql': "INSERT INTO `testapp_location` (`company_id`, `address`) VALUES (1, '123 Test St')",
  'time': '0.000'},
 {'sql': 'SELECT `testapp_company_admins`.`user_id` FROM `testapp_company_admins` WHERE (`testapp_company_admins`.`company_id` = 1  AND `testapp_company_admins`.`user_id` IN (2))',
  'time': '0.000'},
 {'sql': 'INSERT INTO `testapp_company_admins` (`company_id`, `user_id`) VALUES (1, 2)',
  'time': '0.000'},
 {'sql': 'SELECT `auth_user`.`id`, `auth_user`.`username`, `auth_user`.`first_name`, `auth_user`.`last_name`, `auth_user`.`email`, `auth_user`.`password`, `auth_user`.`is_staff`, `auth_user`.`is_active`, `auth_user`.`is_superuser`, `auth_user`.`last_login`, `auth_user`.`date_joined` FROM `auth_user` INNER JOIN `testapp_location_admins` ON (`auth_user`.`id` = `testapp_location_admins`.`user_id`) WHERE `testapp_location_admins`.`location_id` = 1  LIMIT 21',
  'time': '0.000'}]

As you can see the query being run has JOIN testapp_location_admins
where it should be testapp_company_admins.

Two possible work-arounds are, either 1) define 'through' models for each
relation, or 2) set related_name='Company_admins+' and
related_name='Location_admins+' respectively.

I figured out the second work-around by digging around the source, where I saw
there are some cases where this kind of related name is generated internally.
And that any related_name ending with '+' gets special treatment
(is_hidden() returns True)

I was able to trace the issue as far as ManyRelatedManager.get_query_set which
is creating a queryset with a join to the wrong table name. As far as I can tell
the manager and the field itself are all referencing the correct tables and
fields, so I am unsure where this is table name confusion is occurring.

Attachments (1)

ticket_18595_tests.diff (1.2 KB) - added by akaariai 21 months ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 22 months ago by sean@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I have also found that it is necessary to use this: related_name = '%(app_label)s_%(class)s_+' on m2m fields in abstract classes as well, you can not just do related_name = '+' or it will use the wrong table name when you try to do o.m2m.all() (or anything that calls ManyRelatedManager.get_query_set)

comment:2 Changed 21 months ago by akaariai

I can't verify this ticket - having two related_name='+' doesn't validate. A small problematic patch attached.

Changed 21 months ago by akaariai

comment:3 Changed 21 months ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

Accepting per Anssi's comment. See also #15932.

comment:4 Changed 21 months ago by aaugustin

  • Resolution set to invalid
  • Status changed from new to closed

Oops - I read "I can verify" instead of "I can't verify".

This ticket reports a bug after applying a patch that wasn't committed in Django yet -- and probably won't.

Version 0, edited 21 months ago by aaugustin (next)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.