Opened 10 years ago

Closed 10 years ago

Last modified 7 years ago

#7315 closed (fixed)

Forcing Inner join to a nullable foreignkey when it is inherited from an abstract class

Reported by: Marinho Brandão Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


I am having a similar problem to #6981 ticket, but in my case the queryset doesn't uses select_related, and the column is in the inherited abstract class.

class AbstractCrane(models.Model):
    manufacturer = models.ForeignKey(
            limit_choices_to={'is_manufacturer': True},

class WishListCrane(AbstractCrane):

queryset (there is 1 row, but it doesn't is found):

>>> WishListCrane.objects.all()

>>> connection.queries[-1]['sql']
u'SELECT "cranes_wishlistcrane"."id", "cranes_wishlistcrane"."new_manufacturer_id", "cranes_wishlistcrane"."other_manufacturer", "cranes_wishlistcrane"."category_id", "cranes_wishlistcrane"."other_category", "cranes_wishlistcrane"."model", "cranes_wishlistcrane"."lift_capacity", "cranes_wishlistcrane"."boom_length", "cranes_wishlistcrane"."jib_length", "cranes_wishlistcrane"."description", "cranes_wishlistcrane"."user_id", "cranes_wishlistcrane"."require_manufacturer", "cranes_wishlistcrane"."require_categories", "cranes_wishlistcrane"."require_model", "cranes_wishlistcrane"."require_lift_capacity", "cranes_wishlistcrane"."require_boom_length", "cranes_wishlistcrane"."require_jib_length", "cranes_wishlistcrane"."require_description" FROM "cranes_wishlistcrane" INNER JOIN "companies_company" ON ("cranes_wishlistcrane"."new_manufacturer_id" = "companies_company"."id") ORDER BY "companies_company"."name" ASC, "cranes_wishlistcrane"."model" ASC'

I found this part of django/db/models/sql/, and tought maybe it is wrong:

840:         elif promote or outer_if_first:
841:            join_type = self.LOUTER

I modified to:

840:         elif promote or outer_if_first or nullable:
841:            join_type = self.LOUTER

and worked.

I'm nor sure if this is correct for the architecture, because I had no made a better checking, but worked for me

Attachments (1)

7315-fix.diff (568 bytes) - added by Marinho Brandão 10 years ago.

Download all attachments as: .zip

Change History (9)

Changed 10 years ago by Marinho Brandão

Attachment: 7315-fix.diff added

comment:1 Changed 10 years ago by Marinho Brandão

Has patch: set

comment:2 Changed 10 years ago by Alex Gaynor

Do all the tests still pass with this modification?

comment:3 Changed 10 years ago by George Vilches

Keywords: qsrf-cleanup added

comment:4 Changed 10 years ago by Marinho Brandão


I have two sites working on this change since I did it. They has, each one, about 10k page views per day, and they are working well.

But I'm not sure if this change is effective for other situations

comment:5 Changed 10 years ago by Jacob

milestone: 1.0

comment:6 Changed 10 years ago by Malcolm Tredinnick

The example you give here isn't complete (e.g. abstract=True is missing), but looking at the query, it seems there is some default ordering being applied in the Company model. So I suspect this is #7181 in disguise and might be fixed in [7761] (since it's only the ordering table that is incorrectly joined). Please check against that revision and, if the problem still occurs, provide the missing details in the example: right now, nobody but you can recreate the example, since we don't have all the information necessary (and you can probably remove stuff like db_column and limit_choices_to, I suspect, without changing the result, which will make things simpler).

The patch you supply isn't correct because it goes against the very definition of the nullable parameter (the join can be promoted in the future). A join that should be outer right now has promote=True already.

comment:7 Changed 10 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

Closing on the grounds that it's almost certainly a dupe of #7181. Please reopen with a self-contained example if the problem still exists.

comment:8 Changed 7 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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