Opened 14 years ago
Closed 9 years ago
#15804 closed Bug (fixed)
Query lookup types should be scoped to the last joined field's model
Reported by: | Adrian Holovaty | Owned by: | Malcolm Tredinnick |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
This is kind of obscure and best described by example. I have two models:
from django.db import models from django.contrib.gis.db import models as geo_models # Note this is a geo-aware model. class Place(geo_models.Model): geom = geo_models.GeometryField() # Note this is NOT a geo-aware model. class Person(models.Model): name = models.CharField(max_length=50) hometown = models.ForeignKey(Place)
I'd like to be able to do this:
Person.objects.filter(hometown__geom__intersects=some_geometry)
...but that doesn't work. It gives me this error:
django.core.exceptions.FieldError: Join on field 'geom' not permitted. Did you misspell 'intersects' for the lookup type?
This happens because Person
isn't a geo-aware model, so it doesn't know that __intersects
is a valid lookup type. I can fix it by changing Person
to extend django.contrib.gis.db.models
and adding the GeoManager
, but that feels hacky because the Person
model itself doesn't have any geo fields.
The solution could be to change Django's join code so that it looks at the model of the last field in the chain when determining whether a lookup is valid. I haven't looked into how difficult this would be, though. I think it's just enough of an edge case that it wouldn't be worth fixing if it required a ton of refactoring and hoop-jumping. But if it's easy, let's do it.
Change History (3)
comment:1 by , 14 years ago
Description: | modified (diff) |
---|
comment:2 by , 13 years ago
Easy pickings: | unset |
---|---|
Owner: | changed from | to
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
comment:3 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Not sure when this was fixed, but it seems to work at least in 1.8+.
This is pretty much related to the fact that all things linking to GIS models require a GeoManager for just this reason. In my head, it's not amazingly hard to do this and has been a "should get around to that" item for quite a while. We would effectively promote the manager to the correct subclass (which drags in the right QuerySet subclass) as we proceed down the chain. Particularly in the GIS case, this makes things a *lot* easier.
In practice, it may be a little fiddly, but I don't think it requires enormous amounts of internal refactoring. Roughly speaking, an attribute on the manager to say if it needs to be "promoted to" should we enter that class and awareness of that attribute by various clone methods.