Opened 16 years ago

Closed 16 years ago

Last modified 12 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: dev
Severity: Keywords: qsrf-cleanup
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

models.py:

class AbstractCrane(models.Model):
    manufacturer = models.ForeignKey(
            'Company',
            null=True,
            blank=True,
            db_column='new_manufacturer_id',
            limit_choices_to={'is_manufacturer': True},
            )

class WishListCrane(AbstractCrane):
    pass

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/query.py, 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 16 years ago.

Download all attachments as: .zip

Change History (9)

by Marinho Brandão, 16 years ago

Attachment: 7315-fix.diff added

comment:1 by Marinho Brandão, 16 years ago

Has patch: set

comment:2 by Alex Gaynor, 16 years ago

Do all the tests still pass with this modification?

comment:3 by George Vilches, 16 years ago

Keywords: qsrf-cleanup added

comment:4 by Marinho Brandão, 16 years ago

@Alex

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 by Jacob, 16 years ago

milestone: 1.0

comment:6 by Malcolm Tredinnick, 16 years ago

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 by Malcolm Tredinnick, 16 years ago

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 by Jacob, 12 years ago

milestone: 1.0

Milestone 1.0 deleted

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