Opened 6 years ago

Closed 16 months 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 6 years ago.
promote_alias fix and tests

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by ryanbutterfield

promote_alias fix and tests

comment:1 Changed 6 years ago by russellm

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

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 5 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:3 Changed 5 years ago by anonymous

  • Easy pickings unset
  • Resolution set to fixed
  • Status changed from new to closed
  • 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 5 years ago by anonymous

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:5 Changed 5 years ago by aaugustin

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

comment:6 Changed 3 years ago by aaugustin

  • Status changed from reopened to new

comment:7 Changed 16 months ago by timgraham

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

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