Opened 11 years ago

Closed 8 years ago

Last modified 7 years ago

#5537 closed (fixed)

Remove Reverse Lookups on ForeignKeys/ManyToMany

Reported by: David Cramer Owned by: nobody
Component: Documentation Version: master
Severity: Keywords:
Cc: Joel Watts Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


There should be way to remove the addition of a property on models for doing reverse lookups.

Say I have a minor table, that holds statistical information, and it has rows per user. I will never use myuser.statistical_table.all(), so it makes sense to have a way to tell it to not append that object to the instances.

Attachments (1)

relatedname (1).diff (559 bytes) - added by Yeago 8 years ago.
Docfix per russel

Download all attachments as: .zip

Change History (17)

comment:1 Changed 11 years ago by James Bennett

Triage Stage: UnreviewedDesign decision needed

Personally I'd be -1 on this because it would clutter the ORM syntax while only providing noticeable utility in rather extreme use cases.

comment:2 Changed 11 years ago by David Cramer

This could possibly be a large performance gain as well. Maybe it should even be pushed the related_name="" be required to add it to the reverse instances. If you're selecting 100 rows (an normal query), that has 50 accessors on it (I'm sure people attach to "User" just as much as us), it must have some impact on performance to attach 500 instances of a manager to those classes.

comment:3 Changed 11 years ago by Russell Keith-Magee

Triage Stage: Design decision neededAccepted

This would in no way be a performance boost. The accessors for reverse lookups are descriptors; they aren't evaluated until they are used.

However, I'm promoting this to accepted. We don't need to add a keyword to the ORM syntax - we can exploit related_name. Here's the use case, writ large:

class Note(Model)
   text = CharField()

class Author(Model):
   name = CharField()
   name_notes = ManyToManyField(Note, related_name='author_names')
   age = IntegerField()
   age_notes = ManyToManyField(Note, related_name='author_ages')
   address = CharField()
   address_notes = ManyToManyField(Note, related_name='author_addresses')

In this example, related_name is a required field (to avoid namespace collisions), and the names must be unique. This means you have to invent unique (and meaningful) names, even if the related object is of no use.

However, if you allowed related_name=None to mean 'dont install the related object', you could avoid the namespace collision, avoid the need to invent unique names, and clean up the model definition.

comment:4 Changed 11 years ago by James Bennett

Triage Stage: AcceptedDesign decision needed

Russ, are you sure the change is worth the cost? As you've described it, all it does is slightly simplify the case of multiple relations between the same set of models, with no performance gain, and it does so at the cost of a backwards-incompatible change which breaks virtually any model that's ever used a relation and which makes the common case more complex.

comment:5 Changed 11 years ago by Russell Keith-Magee

Ah... yeah... that... oops. Missed the great big honking backwards incompatibility. Scratch that idea :-)

One alternative would be to define a NO_REVERSE string that can act as a marker for those fields you don't want to have reverse relations. related_name=NO_REVERSE isn't quite as clean as related_name=None, but it would avoid the backwards incompatibility problem.

However, I'm happy to go with consensus/BDFL judgement on this. I'm still in favour of the general feature, but obviously not at the expense of a clean interface. I also wouldn't lose any sleep if it wasn't implemented, because there _is_ a workaround.

comment:6 Changed 11 years ago by Noam Raphael <spam.noam@…>

I just wanted to say that I would like this feature too - I don't like to invent unused names.

By the way, perhaps related_name=None can be made backwards-compatible. I think that when people write models that depend on the automatic related name, they don't write related_name=None - they just don't write anything. So you can make a constant related_name=AUTOMATIC, which will be the default, and related_name=None will make no related field.


comment:7 Changed 11 years ago by Malcolm Tredinnick

I'm -1 on this. There's no demonstrated gain, either performance wise or from a technical perspective. Not worth it.

comment:8 Changed 11 years ago by David Cramer

I just want this so it doesn't clutter up all of our exception messages when we're debugging :)

Whether its a performance gain or not (I'm guessing it is, but miniscule), that's another story.

comment:9 Changed 10 years ago by jgeewax

Not sure if this helps, but I had a similar problem and used the following subclass so that I didn't need to define related names for the reverse look ups that didn't matter:

from django.db import models

class NonReversibleManyToManyField(models.ManyToManyField):

_relation_counter = 0

def generate_related_name(cls):

cls._relation_counter += 1
return "anonymous_relation_%s" % cls._relation_counter

def contribute_to_related_class(self, cls, related):

self.rel.related_name = NonReversibleManyToManyField.generate_related_name()
return super(NonReversibleManyToManyField, self).contribute_to_related_class(cls, related)

This seems to pass validation, and the direct look up seems to work fine, it just creates anonymous reverse lookups "anonymous_relation_N" for all the others.

Not sure if this is perfect but it seemed to solve the problem for me.

comment:10 Changed 9 years ago by Joel Watts

Cc: Joel Watts added

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

Component: Database layer (models, ORM)Documentation
Triage Stage: Design decision neededAccepted

Accepted as a documentation fix; this is actually possible right now as long as you start your related_name with a "+".

Changed 8 years ago by Yeago

Attachment: relatedname (1).diff added

Docfix per russel

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

milestone: 1.3
Triage Stage: AcceptedReady for checkin

comment:13 Changed 8 years ago by Simon Meers

I think you mean end related_name with a '+'

comment:14 Changed 8 years ago by Simon Meers

Resolution: fixed
Status: newclosed

(In [14049]) Fixed #5537 -- document trailing '+' on related_name for supressing backward relation.

Thanks to dcramer for the report, and Russ for pointing out the workaround.

comment:15 Changed 8 years ago by Simon Meers

(In [14050]) [1.2.X] Fixed #5537 -- document trailing '+' on related_name for supressing backward relation.

Thanks to dcramer for the report, and Russ for pointing out the workaround.

Backport of r14049 from trunk.

comment:16 Changed 7 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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