Opened 9 years ago

Closed 4 years ago

#14601 closed Bug (fixed)

ValuesQuerySet join types not being promoted

Reported by: ryanbutterfield Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: ValuesQuerySet promote_alias
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


In the following test case the join type of the User model isn't promoted (to a LEFT OUTER JOIN) as it should be when using a ValuesQuerySet. This results in nothing being returned when the Item.person ForeignKey is null.

class User(models.Model):
    first_name = models.CharField(max_length=100)
    last_name = models.CharField(max_length=100)
class Person(models.Model):
    user = models.ForeignKey(User)

class Item(models.Model):
    person = models.ForeignKey(Person, null=True)
    amount = models.IntegerField()

To replicate the following is required:

  1. Three (or more) models deep linked by ForeignKey's.
  2. The first model's (Item) ForeignKey should allow null values.
  3. The query should reference the second model (Person), but not the third (User), before conversion to a ValuesQuerySet.
  4. The values list (that is passed to ValueQuerySet) should reference a field in the third model (User).

The following snippet shows what happens currently. Note that the Q objects aren't required, they are just for satisfying point 3 above.

>>> Item.objects.create(person=None, amount=123)
<Item: Item object>

>>> Item.objects.values('amount', 'person__user__first_name', 'person__user__last_name')
[{'amount': 123, 'person__user__last_name': None, 'person__user__first_name': None}]

>>> Item.objects.filter(Q(person__isnull=True) | Q(person__isnull=False))
[<Item: Item object>]

>>> Item.objects.filter(Q(person__isnull=True) | Q(person__isnull=False)).values('amount', 'person__user__first_name', 'person__user__last_name')

What happens:

  • When the filter is constructed, a query.alias_map entry is created for the Person model. It is NULLABLE so the join type is set to LOUTER which is correct.
  • During the conversion to a ValuesQuerySet, promote_alias_chain is called on the fields from the User model (eg. person__user__first_name or person__user__last_name).
  • Because Person had been previously promoted promote_alias returns False which causes promote_alias_chain to not promote the User model.

What should happen:

  • promote_alias should return True if it promotes the join or if the join has previously been promoted, so that promote_alias_chain can promote the remainder of the chain correctly.

The included diff provides a simple fix to promote_alias and a test case that tests the fix.

Attachments (1)

promote_alias.diff (2.6 KB) - added by ryanbutterfield 9 years ago.
promote_alias fix and tests

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by ryanbutterfield

Attachment: promote_alias.diff added

promote_alias fix and tests

comment:1 Changed 8 years ago by Russell Keith-Magee

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Tests need to be updated to be unittests. Preferably, they should also be integrated into an existing test app, rather than starting a whole new test app. The regressiontest/queries test app would seem appropriate.

comment:2 Changed 8 years ago by Julien Phalip

Severity: Normal
Type: Bug

comment:3 Changed 7 years ago by anonymous

Easy pickings: unset
Resolution: fixed
Status: newclosed
UI/UX: unset

This bug still exists. The patch works fine for me, any reason there's been no activity on this for 9 months?

comment:4 Changed 7 years ago by anonymous

Resolution: fixed
Status: closedreopened

Accidently closed, oops. Should anonymous users really have permissions to do that? :/

comment:5 Changed 7 years ago by Aymeric Augustin

If you want to see this move forward, please Russel's comment above.

comment:6 Changed 6 years ago by Aymeric Augustin

Status: reopenednew

comment:7 Changed 4 years ago by Tim Graham

Resolution: fixed
Status: newclosed

It seems this was fixed in 1.5 as the tests from the patch fail in 1.4 but pass on 1.5 and later.

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