Code

Opened 7 years ago

Closed 6 years ago

#4306 closed (fixed)

'to_field' breaks lookups that span relationships

Reported by: shwag Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qs-rf-fixed
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by mtredinnick)

class Website(models.Model):
    id = models.IntegerField(primary_key=True)
    dfp_id = models.IntegerField()
    name = models.CharField(maxlength=300)
    url = models.CharField(maxlength=150)

class Report(models.Model):
    id = models.IntegerField(primary_key=True)
    site = models.ForeignKey(Website, to_field='dfp_id')

444549 is a valid dfp_id in the Website model.

Report.objects.filter(site=444549)  <-- Returns results. good.
Report.objects.filter(site__id=444549)  <-- Returns results, but it shouldn't.
Report.objects.filter(site__dfp_id=444549)  <-- No results, but it should.

130 is the valid id for a Website.

Report.objects.filter(site__id=130) <- No results, no work around.
Report.objects.filter(site__name='Validname') <- Also no results.
Report.objects.filter(site__url='validurl.com') <- Also no results

Attachments (0)

Change History (9)

comment:1 Changed 7 years ago by Steven Wagner <stevenwagner@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
class Website(models.Model):
    id = models.IntegerField(primary_key=True)
    dfp_id = models.IntegerField()
    name = models.CharField(maxlength=300)
    url = models.CharField(maxlength=150)

class Report(models.Model):
    id = models.IntegerField(primary_key=True)
    site = models.ForeignKey(Website, to_field='dfp_id')

444549 is a valid dfp_id in the Website model.

Report.objects.filter(site=444549)  <-- Returns results. good.
Report.objects.filter(site__id=444549)  <-- Returns results, but it shouldn't.
Report.objects.filter(site__dfp_id=444549)  <-- No results, but it should.

130 is the valid id for a Website.
Report.objects.filter(site__id=130) <- No results, no work around.
Report.objects.filter(site__name='Validname') <- Also no results.
Report.objects.filter(site__url='validurl.com') <- Also no results

comment:2 Changed 7 years ago by Steven Wagner <stevenwagner@…>

I applied the patch from bug 4088 (http://code.djangoproject.com/attachment/ticket/4088/newcolumn.diff), and that fixed one case.
Report.objects.filter(sitedfp_id=444549). It fixes the inner join.

FROM reports
INNER JOIN websites AS reports__site ON reports.site_id = reports__site.id
WHERE (reports__site.dfp_id = 444549)

FROM reports
INNER JOIN websites AS reports__site ON reports.site_id = reports__site.dfp_id
WHERE (reports__site.dfp_id = 444549)

It fixed all cases I defined above except for one.

Report.objects.filter(siteid=444549) <-- Returns results, but it shouldn't.

when I look in my mysql.log, I find the query it tried does not use a join at all even though it should.
Select ..
FROM reports WHERE (reports.site_id = 130)

comment:3 Changed 7 years ago by Steven Wagner <stevenwagner@…>

  • Version changed from 0.96 to SVN
Argh, formatting. 
I applied the patch from bug 4088 (http://code.djangoproject.com/attachment/ticket/4088/newcolumn.diff), and that fixed ALMOST ALL cases.
This is fixed, Report.objects.filter(site__dfp_id=444549). The inner join is correct now.

FROM `reports` 
INNER JOIN `websites` AS `reports__site` ON `reports`.`site_id` = `reports__site`.`id` 
WHERE (`reports__site`.`dfp_id` = 444549)

FROM `reports` 
INNER JOIN `websites` AS `reports__site` ON `reports`.`site_id` = `reports__site`.`dfp_id` 
WHERE (`reports__site`.`dfp_id` = 444549)

This only case that is still broken is if trying to access the related column when the column name is 'id'

Report.objects.filter(site__id=444549)  <-- Returns results, but it shouldn't.

when I look in my mysql.log, I find the query it tried does not use a join at all even though it should.
Select ..
FROM `reports` WHERE (`reports`.`site_id` = 130)

comment:4 Changed 7 years ago by Steven Wagner <stevenwagner@…>

This comment will describe the only remaining problem about the patch from bug 4088 is applied.

Here is the case that is still broken in Django.

class Website(models.Model):
    id = models.IntegerField(primary_key=True)
    dfp_id = models.IntegerField()
    name = models.CharField(maxlength=300)
    url = models.CharField(maxlength=150)

class Report(models.Model):
    id = models.IntegerField(primary_key=True)
    site = models.ForeignKey(Website, to_field='dfp_id')



Report.objects.filter(site=444549)
  SELECT ...
  FROM `reports` WHERE (`reports`.`site_id` = 444549)   (Correct)

Report.objects.filter(site__id=444549)
  SELECT ...
  FROM `reports` WHERE (`reports`.`site_id` = 444549)   (Incorrect)

Report.objects.filter(site__dfp_id=444549)
  SELECT ...
  FROM `reports` INNER JOIN `websites` AS `reports__site`
    ON `reports`.`site_id` = `reports__site`.`dfp_id` 
  WHERE (`reports__site`.`dfp_id` = 444549)       (Correct)

comment:5 Changed 7 years ago by Steven Wagner <stevenwagner@…>

The first query looks fine.

The second query generated is the same as the first, which is incorrect. It should be generating a join to the website table on the id column similar to query 3.

The third query looks fine, and achieves the same result as the first but also uses a join.

comment:6 Changed 7 years ago by mtredinnick

  • Description modified (diff)
  • Triage Stage changed from Unreviewed to Accepted

Fixed description formatting.

comment:7 Changed 7 years ago by mtredinnick

  • Keywords qs-rf added

comment:8 Changed 7 years ago by mtredinnick

  • Keywords qs-rf-fixed added; qs-rf removed

comment:9 Changed 6 years ago by mtredinnick

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

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing
code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658

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.