Some complex lookups with Q objects (or-style) don't return all matching model objects.

[brooks.travis@…]

When using Q objects to make some complex queries, the resulting queryset does not contain all expected objects. Here is an example case:

Models:

class ExtendedUser(User):
	"""
	This class inherits (MTI-style) from auth.models.User to add extra fields, and is the parent class 
	for the SupportStaff and Client classes to keep things DRY.
	"""
	parent = models.OneToOneField(User, parent_link=True)
	middle_initial = models.CharField(max_length=10, blank=True)
	title = models.CharField(max_length=50, blank=True)
	office_building = models.ForeignKey("Building", null=True, blank=True)
	office_number = models.CharField(max_length=15, blank=True)
	phone = models.PhoneNumberField(blank=True)
	twitter = models.CharField(max_length=50, blank=True)
	pownce = models.CharField(max_length=50, blank=True)
	aim = models.CharField(max_length=50, blank=True)
	msn = models.CharField(max_length=50, blank=True)
	yahoo = models.CharField(max_length=50, blank=True)
	
	def get_full_name(self):
		if self.middle_initial:
			return u"%s %s %s" % (self.first_name, self.middle_initial, self.last_name)
		else:
			return super(ExtendedUser, self).get_full_name()
			
	def __unicode__(self):
		return u"%s" % self.get_full_name()
	
	class Meta:
		ordering = ('last_name', 'first_name')
		verbose_name = u'Extended User'
		verbose_name_plural = u'Extended Users'

class SupportStaff(ExtendedUser):
	"""
	This class inherits from the model class ExtendedUser (itself a subclass of auth User model). 
	This should only be used for people who are going to provide end-user support services.
	"""
	parent = models.OneToOneField(ExtendedUser, parent_link=True)
	coverage = models.ManyToManyField("OrgUnit", related_name="support_staff_coverage", null=True, blank=True)
	expertise = models.ManyToManyField("SpecialSkill", related_name="support_staff_expertise", null=True, blank=True)
	
	def __unicode__(self):
		return u"%s" % self.get_full_name()

class SupportTicket(models.Model):
	call_source = models.ForeignKey("CallSource", null=True, blank=True)
	client = models.ManyToManyField(Client)
	call_description = models.TextField("Call Description", max_length=500)
	call_type = models.ManyToManyField("CallType")
	equipment_tag = models.CharField(max_length=50, null=True, blank=True)
	priority = models.CharField(choices=PRIORITY_LEVELS, default=3, max_length=2)
	target_date = models.DateTimeField("Target Date and Time", null=True, blank=True)
	call_status = models.ForeignKey("CallStatus")
	cause = models.ManyToManyField("Cause", null=True, blank=True)
	solution = models.TextField(max_length=500, blank=True)
	date_created = models.DateTimeField(default=datetime.datetime.now, editable=False)
	created_by = models.ForeignKey(SupportStaff, related_name='ticket_created')
	date_modified = models.DateTimeField(null=True, blank=True, editable=False)
	last_modified_by = models.ForeignKey(SupportStaff, related_name='ticket_modified_by', null=True, blank=True)
	date_closed = models.DateTimeField(null=True, blank=True)
	closed_by = models.ForeignKey(User, related_name='ticket_closed_by', null=True, blank=True)

class Assignment(models.Model):
	support_ticket = models.ForeignKey(SupportTicket, edit_inline=models.STACKED, num_in_admin=1, related_name='assignment_made')
	group_assigned = models.ForeignKey(Group, core=True)
	group_notified = models.NullBooleanField(editable=False)
	support_staff_assigned = models.ForeignKey(SupportStaff, related_name='staff_assignment', null=True, blank=True)
	support_staff_notified = models.NullBooleanField(editable=False)
	notes = models.TextField(max_length=500, blank=True)
	acknowledge = models.BooleanField(default=False)
	date_created = models.DateTimeField(default=datetime.datetime.now, editable=False)
	created_by = models.ForeignKey(SupportStaff, related_name='assigned_by')
	date_modified = models.DateTimeField(default=datetime.datetime.now, editable=False)
	last_modified_by = models.ForeignKey(SupportStaff, related_name='assignment_modified_by', null=True, blank=True)
	date_acknowledged = models.DateTimeField(null=True, blank=True, editable=False)

Queries:

>>> t4 = SupportTicket.objects.filter(assignment_made__group_assigned__user=u)
>>> t5 = SupportTicket.objects.filter(created_by__parent__parent=u)
>>> t6 = SupportTicket.objects.filter(Q(assignment_made__group_assigned__user=u)|Q(created_by__parent__parent=u)).distinct()
>>> t4
[<SupportTicket: 3>, <SupportTicket: 2>, <SupportTicket: 1>]
>>> t5
[<SupportTicket: 5>, <SupportTicket: 3>, <SupportTicket: 2>, <SupportTicket: 1>]
>>> t6
[<SupportTicket: 3>, <SupportTicket: 2>, <SupportTicket: 1>]
>>> str(SupportTicket.objects.filter(Q(assignment_made__group_assigned__user=u)|Q(created_by__parent__parent=u)).distinct().query)
'SELECT DISTINCT "tickets_supportticket"."id", "tickets_supportticket"."call_source_id", "tickets_supportticket"."call_description", "tickets_supportticket"."equipment_tag", "tickets_supportticket"."priority", "tickets_supportticket"."target_date", "tickets_supportticket"."call_status_id", "tickets_supportticket"."solution", "tickets_supportticket"."date_created", "tickets_supportticket"."created_by_id", "tickets_supportticket"."date_modified", "tickets_supportticket"."last_modified_by_id", "tickets_supportticket"."date_closed", "tickets_supportticket"."closed_by_id" FROM "tickets_supportticket" LEFT OUTER JOIN "tickets_assignment" ON ("tickets_supportticket"."id" = "tickets_assignment"."support_ticket_id") INNER JOIN "auth_group" ON ("tickets_assignment"."group_assigned_id" = "auth_group"."id") LEFT OUTER JOIN "auth_user_groups" ON ("auth_group"."id" = "auth_user_groups"."group_id") WHERE ("auth_user_groups"."user_id" = 6  OR "tickets_supportticket"."created_by_id" = 6 ) ORDER BY "tickets_supportticket"."date_created" DESC'

[brooks.travis@…]

As an update, I've managed to suss out that that this only occurs when there is no Assignment instance attached to the "missing" SupportTicket instance. If I create an Assignment with the missing instance as it's foreign key, things behave as expected.

[Michael Radziej]

How shall we check this? Your models refer to several undefined classes (Client, CallSource, etc.), and I cannot even run manage.py validate

Please add all the other classes, too, and re-open the ticket then.

[brooks.travis@…]

Um, just remove the fields references to models that I didn't include. They're not being queried against, anyway. I guess it was my bad for assuming that was a simple thing for someone to do. I was trying to get the ticket posted and I was in a hurry. However, as I have a bit more time, now, I'll do it for you:

class ExtendedUser(User):
	"""
	This class inherits (MTI-style) from auth.models.User to add extra fields, and is the parent class 
	for the SupportStaff and Client classes to keep things DRY.
	"""
	parent = models.OneToOneField(User, parent_link=True)
	middle_initial = models.CharField(max_length=10, blank=True)
	title = models.CharField(max_length=50, blank=True)
	office_building = models.ForeignKey("Building", null=True, blank=True)
	office_number = models.CharField(max_length=15, blank=True)
	phone = models.PhoneNumberField(blank=True)
	twitter = models.CharField(max_length=50, blank=True)
	pownce = models.CharField(max_length=50, blank=True)
	aim = models.CharField(max_length=50, blank=True)
	msn = models.CharField(max_length=50, blank=True)
	yahoo = models.CharField(max_length=50, blank=True)
	
	def get_full_name(self):
		if self.middle_initial:
			return u"%s %s %s" % (self.first_name, self.middle_initial, self.last_name)
		else:
			return super(ExtendedUser, self).get_full_name()
			
	def __unicode__(self):
		return u"%s" % self.get_full_name()
	
	class Meta:
		ordering = ('last_name', 'first_name')
		verbose_name = u'Extended User'
		verbose_name_plural = u'Extended Users'
	
	class Admin:
		pass

class SupportStaff(ExtendedUser):
	"""
	This class inherits from the model class ExtendedUser (itself a subclass of auth User model). 
	This should only be used for people who are going to provide end-user support services.
	"""
	parent = models.OneToOneField(ExtendedUser, parent_link=True)
	coverage = models.CharField(max_length=20, blank=True)
	expertise = models.CharField(max_length=20, blank=True)
	
	def __unicode__(self):
		return u"%s" % self.get_full_name()
	
	class Meta:
		ordering = ('last_name', 'first_name')
		verbose_name = u'Support Staff'
		verbose_name_plural = u'Support Staff'
	
	class Admin:
		pass


class Client(ExtendedUser):
	"""
	This class inherits from the model class ExtendedUser (itself a subclass of auth User model).
	All user imported into the system as clients should be edited through this model.
	"""
	parent = models.OneToOneField(ExtendedUser, parent_link=True)
	unit = models.CharField(max_length=20, blank=True)
	client_type = models.CharField(max_length=50, blank=True)
	
	def __unicode__(self):
		return u"%s" % self.get_full_name()

	def get_admin_url(self):
		pk = getattr(self, self._meta.pk.attname)
		return ('%s/admin/%s/%s/%s' % (settings.URL_PREFIX, self._meta.app_label, self._meta.module_name, pk))
			
	class Meta:
		ordering = ('last_name', 'first_name')
	
	class Admin:
		pass

class SupportTicket(models.Model):
	call_source = models.ForeignKey("CallSource", null=True, blank=True)
	client = models.ManyToManyField(Client)
	call_description = models.TextField("Call Description", max_length=500)
	equipment_tag = models.CharField(max_length=50, null=True, blank=True)#!! **FEATURE** Tie this to inventory system, in some way, probably via a templatetag.
	priority = models.CharField(choices=PRIORITY_LEVELS, default=3, max_length=2)
	target_date = models.DateTimeField("Target Date and Time", null=True, blank=True)
	solution = models.TextField(max_length=500, blank=True)
	date_created = models.DateTimeField(default=datetime.datetime.now, editable=False)
	created_by = models.ForeignKey(SupportStaff, related_name='ticket_created')
	date_modified = models.DateTimeField(null=True, blank=True, editable=False)
	last_modified_by = models.ForeignKey(SupportStaff, related_name='ticket_modified_by', null=True, blank=True)
	date_closed = models.DateTimeField(null=True, blank=True)
	closed_by = models.ForeignKey(User, related_name='ticket_closed_by', null=True, blank=True)

class Assignment(models.Model):
	support_ticket = models.ForeignKey(SupportTicket, edit_inline=models.STACKED, num_in_admin=1, related_name='assignment_made')
	group_assigned = models.ForeignKey(Group)
	group_notified = models.NullBooleanField(editable=False)
	support_staff_assigned = models.ForeignKey(SupportStaff, related_name='staff_assignment', null=True, blank=True)
	support_staff_notified = models.NullBooleanField(editable=False)
	notes = models.TextField(max_length=500, blank=True)
	acknowledge = models.BooleanField(default=False)
	date_created = models.DateTimeField(default=datetime.datetime.now, editable=False)
	created_by = models.ForeignKey(SupportStaff, related_name='assigned_by')
	date_modified = models.DateTimeField(default=datetime.datetime.now, editable=False)
	last_modified_by = models.ForeignKey(SupportStaff, related_name='assignment_modified_by', null=True, blank=True)
	date_acknowledged = models.DateTimeField(null=True, blank=True, editable=False)

[Michael Radziej]

Yes, this is a bug, the one INNER JOIN should be a LEFT OUTER JOIN. Thanks!

I'll attach a minimal test case (as patch).

[Michael Radziej]

patch with a test case

[Michael Radziej]

Ahem, just to make it clear: My patch contains only the test case, not a fix.

[Michael Radziej]

If you don't use Q objects, you get the expected behaviour, i.e.:

t6 = (SupportTicket.objects.filter(assignment_made__group_assigned__user=u) | SupportTicket.objects.filter(created_by__parent__parent=u)).distinct()

(At least this worked in my test case.)

I recall remotely that there's a subtle semantic difference when you use Q objects: It means that all the conditions must apply to the same object. It should not apply here in my opinion, but I'm not really sure. Malcolm?

[Malcolm Tredinnick]

There are differences between a single filter() call (which I guess is what you mean by merging Q-objects) and separate calls, which probably includes merging querysets. That difference only applies when filtering multi-valued fields (this is documented in the db-api for filter()). I haven't had time to wrap my head around this particular query yet, though, so I have no idea if it's relevant here. I'll get to it soon.

[Michael Radziej]

Replying to mtredinnick:

Ah, I found the section about "SPANNING MULTI-VALUED RELATIONSHIPS". That's what I had in mind.

Perhaps I'm still a bit dumb (heck, no coffee yet), but I cannot figure out what this is means in the OR case (while the AND case is clear to me).

[Malcolm Tredinnick]

(In [8107]) Fixed #7872 -- Fixed a missed case of promoting table joins when using
disjunctive filters. Thanks to Michael Radziej for the failing test case.
problem.

[Jacob]

Milestone 1.0 deleted