Code

Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#8036 closed (fixed)

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

Reported by: mrmachine Owned by: mtredinnick
Component: Database layer (models, ORM) Version: master
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: UI/UX:

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 mrmachine 6 years ago.
Client.country wasn't essential to the tests.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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

comment:2 Changed 6 years ago by mtredinnick

  • Owner changed from nobody to mtredinnick

comment:3 Changed 6 years ago by mtredinnick

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

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 Changed 6 years ago by mrmachine

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 Changed 6 years ago by mtredinnick

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

Changed 6 years ago by mrmachine

Client.country wasn't essential to the tests.

comment:6 Changed 6 years ago by mrmachine

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 Changed 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

(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 Changed 5 years ago by sunrise

  • Resolution fixed deleted
  • Status changed from closed to reopened
This works ok for me.. but It probably needs feedback. djangoproject Air Jordan

comment:9 Changed 5 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:10 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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.