Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#8036 closed (fixed)

select_related() yields unexpected results with certain combinations of *fields

Reported by: Tai Lee Owned by: Malcolm Tredinnick
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: select_related
Cc: real.human@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When passing certain combinations of fields (vs depth or no arguments) to select_related(), I get an unexpected model instance (should be None) for a ForeignKey field which has null=True.

Given the following models:

class Country(models.Model):
	created = turbia_models.DateTimeField(auto_now_add=True)
	modified = turbia_models.DateTimeField(auto_now=True)
	name = turbia_models.CharField(max_length=50, unique=True)
	abbreviation = turbia_models.CharField(max_length=10, unique=True)

	def __unicode__(self):
		return self.name

class State(models.Model):
	created = turbia_models.DateTimeField(auto_now_add=True)
	modified = turbia_models.DateTimeField(auto_now=True)
	country = models.ForeignKey(Country)
	name = turbia_models.CharField(max_length=50)
	abbreviation = turbia_models.CharField(max_length=10)

	def __unicode__(self):
		return self.name

class ClientStatus(models.Model):
	created = turbia_models.DateTimeField(auto_now_add=True)
	modified = turbia_models.DateTimeField(auto_now=True)
	name = turbia_models.CharField(max_length=50, unique=True)

	def __unicode__(self):
		return self.name

class Client(models.Model):
	created = turbia_models.DateTimeField(auto_now_add=True)
	modified = turbia_models.DateTimeField(auto_now=True)
	code = turbia_models.CharField(max_length=5, unique=True)
	name = turbia_models.CharField(max_length=50, unique=True)
	street_address_1 = turbia_models.CharField(max_length=50, null=True)
	street_address_2 = turbia_models.CharField(max_length=50, blank=True, null=True)
	suburb = turbia_models.CharField(max_length=50, null=True, verbose_name='town / suburb')
	city = turbia_models.CharField(max_length=50, blank=True, null=True)
	state = models.ForeignKey(State, null=True)
	post_code = turbia_models.CharField(max_length=10, null=True)
	country = models.ForeignKey(Country, null=True)
	telephone = turbia_models.CharField(max_length=20, null=True)
	fax = turbia_models.CharField(max_length=20, blank=True, null=True)
	web_site = models.URLField(blank=True, null=True, verify_exists=False)
	status = models.ForeignKey(ClientStatus, null=True)

	def __unicode__(self):
		return self.name

class JobStatus(models.Model):
	created = turbia_models.DateTimeField(auto_now_add=True)
	modified = turbia_models.DateTimeField(auto_now=True)
	name = turbia_models.CharField(max_length=50, unique=True)
	sequence = models.IntegerField(default=0)

	def __unicode__(self):
		return self.name

class Job(models.Model):
	created = turbia_models.DateTimeField(auto_now_add=True)
	modified = turbia_models.DateTimeField(auto_now=True)
	client = models.ForeignKey(Client)
	status = models.ForeignKey(JobStatus, null=True)
	type = models.ForeignKey(JobType, null=True)
	name = turbia_models.CharField(max_length=50)
	description = turbia_models.TextField(blank=True, null=True)
	people = models.ManyToManyField(Person, blank=True)

The following results are correct when using select_related():

>>> j = Job.objects.filter(pk=1264).select_related()[0]
>>> j.client
<Client: Unilever>
>>> j.client.status
<ClientStatus: active>
>>> j.status
>>> type(j.status)
<type 'NoneType'>

But when using select_related('client__state__country', 'client__status', 'status') I get the following unexpected results:

>>> j = Job.objects.filter(pk=1264).select_related('client__state__country', 'client__status', 'status')[0]
>>> j.client
<Client: Unilever>
>>> j.client.status
>>> type(j.client.status)
<type 'NoneType'>
>>> j.status
Traceback (most recent call last):
  File "<console>", line 1, in ?
  File "/Users/tailee/3030/thirty30/django/db/models/base.py", line 243, in __repr__
    return smart_str(u'<%s: %s>' % (self.__class__.__name__, unicode(self)))
TypeError: coercing to Unicode: need string or buffer, datetime.datetime found
>>> type(j.status)
<class 'turbia.apps.smooth.models.JobStatus'>
>>> j.status.pk
>>> j.status.created
1
>>> j.status.modified
datetime.datetime(2008, 7, 25, 16, 52, 5, 753919)
>>> j.status.name
datetime.datetime(2008, 7, 25, 16, 52, 5, 753919)
>>> j.status.sequence
u'active'

It looks like j.status has taken properties from j.client.status, and mixed them up (e.g. j.status.sequence is the same as j.client.status.name in the expected results, possibly as a result of incorrect SQL when passing fields that are more than one relation away and have null=True and/or multiple ForeignKey fields named status (in different models). If I don't pass client__state__country to select_related, it works. If I assign a state to the client in question, it works. I'm having trouble writing a test to reproduce this outside of the real-world models above.

Here is the SQL for the non-functioning queryset:

>>> str(Job.objects.filter(pk=1264).select_related('client__state__country', 'client__status', 'status').query)
'SELECT "smooth_job"."id", "smooth_job"."created", "smooth_job"."modified", "smooth_job"."client_id", "smooth_job"."status_id", "smooth_job"."type_id", "smooth_job"."name", "smooth_job"."description", "smooth_client"."id", "smooth_client"."created", "smooth_client"."modified", "smooth_client"."code", "smooth_client"."name", "smooth_client"."street_address_1", "smooth_client"."street_address_2", "smooth_client"."suburb", "smooth_client"."city", "smooth_client"."state_id", "smooth_client"."post_code", "smooth_client"."country_id", "smooth_client"."telephone", "smooth_client"."fax", "smooth_client"."web_site", "smooth_client"."status_id", "smooth_state"."id", "smooth_state"."created", "smooth_state"."modified", "smooth_state"."country_id", "smooth_state"."name", "smooth_state"."abbreviation", "smooth_country"."id", "smooth_country"."created", "smooth_country"."modified", "smooth_country"."name", "smooth_country"."abbreviation", "smooth_clientstatus"."id", "smooth_clientstatus"."created", "smooth_clientstatus"."modified", "smooth_clientstatus"."name", "smooth_jobstatus"."id", "smooth_jobstatus"."created", "smooth_jobstatus"."modified", "smooth_jobstatus"."name", "smooth_jobstatus"."sequence" FROM "smooth_job" INNER JOIN "smooth_client" ON ("smooth_job"."client_id" = "smooth_client"."id") LEFT OUTER JOIN "smooth_state" ON ("smooth_client"."state_id" = "smooth_state"."id") LEFT OUTER JOIN "smooth_country" ON ("smooth_state"."country_id" = "smooth_country"."id") LEFT OUTER JOIN "smooth_clientstatus" ON ("smooth_client"."status_id" = "smooth_clientstatus"."id") LEFT OUTER JOIN "smooth_jobstatus" ON ("smooth_job"."status_id" = "smooth_jobstatus"."id") WHERE "smooth_job"."id" = 1264  ORDER BY "smooth_job"."id" DESC'

Attachments (1)

8036-tests-r8571.diff (2.0 KB ) - added by Tai Lee 16 years ago.
Client.country wasn't essential to the tests.

Download all attachments as: .zip

Change History (11)

comment:1 by Jacob, 16 years ago

Triage Stage: UnreviewedAccepted

I suspect this is a duplicate of #8106, but I'm not totally sure.

comment:2 by Malcolm Tredinnick, 16 years ago

Owner: changed from nobody to Malcolm Tredinnick

comment:3 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: newclosed

I've tried pretty hard to reproduce this, but I can't. It's quite possible that this has been fixed with one of the changes committed since you filed the code (you don't mention the subversion revision or release you were testing against).

It's not possible to use your example exactly, since it refers to models that don't exist in the sample code.

If you are still seeing the problem with current trunk, please create a small patch against tests/regressiontests/select_related_regress/models.py that fails. The current example also suffers from having a lot of fields that are almost certainly not relevant, so once you have a failing test case, please remove as many fields as possible so that we can focus on the critical pieces.

In the meantime, I'm going to mark this as fixed, because I think that's what's happened.

comment:4 by Tai Lee, 16 years ago

Resolution: fixed
Status: closedreopened

The problem still exists in [8571]. I've attached tests with as few models and fields as possible. The tests show that select_related() (without *fields) and select_related('state__country') (with *fields) work, but select_related('state__country', 'status') (certain combination of *fields) fails.

18:02:28 tailee@tetsuo ~/Subversion/django/django$ dpath ../tests/runtests.py --settings=django.settings select_related_regress
======================================================================
FAIL: Doctest: regressiontests.select_related_regress.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/tailee/Subversion/django/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for regressiontests.select_related_regress.models.__test__.API_TESTS
  File "/Users/tailee/Subversion/django/tests/regressiontests/select_related_regress/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/Users/tailee/Subversion/django/tests/regressiontests/select_related_regress/models.py", line ?, in regressiontests.select_related_regress.models.__test__.API_TESTS
Failed example:
    Client.objects.select_related('state__country', 'status').latest('id').status
Expected:
    <ClientStatus: ClientStatus object>
Got nothing


----------------------------------------------------------------------
Ran 1 test in 0.024s

FAILED (failures=1)

comment:5 by Malcolm Tredinnick, 16 years ago

Thanks for the test patch. Now I can try to work out what's going on.

by Tai Lee, 16 years ago

Attachment: 8036-tests-r8571.diff added

Client.country wasn't essential to the tests.

comment:6 by Tai Lee, 16 years ago

Looks like Client.country wasn't essential to the test, so I've removed that field from the model and added all combinations of state, state__country, and status to the test (only select_related('state__country', 'status') fails).

comment:7 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: reopenedclosed

(In [8598]) Fixed #8036 -- Fixed a case when attempting to traverse non-existent related
instances. We weren't skipping the correct output columns before processing
subsequent existing related instances. Thanks to mrmachine for the test case.

comment:8 by sunrise, 16 years ago

Resolution: fixed
Status: closedreopened
This works ok for me.. but It probably needs feedback. djangoproject Air Jordan

comment:9 by Karen Tracey, 16 years ago

Resolution: fixed
Status: reopenedclosed

comment:10 by Jacob, 13 years ago

milestone: 1.0

Milestone 1.0 deleted

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