Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#12344 closed (fixed)

select_related() uses always QuerySet and Query classes from originating Model

Reported by: Jani Tiainen Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: query orm
Cc: Erin Kelly, Matt Boersma Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When having two models:

class ModelNormal(models.Model):
    geo = models.ForeignKey('ModelGeo')

class ModelGeo(models.Model):
    geom = models.PointField()
    objects = models.GeoManager()

When querying:

ModelNormal.objects.all().select_related()

Accessing objects causes traceback because Django used Manager instead of GeoManager for GeoModel instance(s).

Further investigation indicated that QSL queries didn't have any decorators that GeoManager does for geometry fields needed in Oracle.

Attachments (4)

select_related_fix_v1.diff (900 bytes ) - added by jbronn 14 years ago.
select_related_fix_v2.diff (1.9 KB ) - added by jbronn 14 years ago.
issue_12344.tar.gz (4.9 KB ) - added by Jani Tiainen 14 years ago.
Sample case when select_related() fails to use custom compiler
custom_backend_example.tar.gz (3.3 KB ) - added by anonymous 14 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by Erin Kelly, 14 years ago

Cc: Erin Kelly Matt Boersma added

comment:2 by Jani Tiainen, 14 years ago

Component: GISDatabase layer (models, ORM)
Keywords: gis Oracle removed
Summary: Mixing geo and nongeo with select_related() doesn't work with Oracleselect_related() uses always QuerySet and Query classes from originating Model

When you have models just like in description and both have different, custom QuerySet containing managers using .select_related() fails.

This is because Django assumes that all related models uses same QuerySet (and Query) as originating table, in case of example ModelNormal.

This is most visible with GeoDjango models since many of them uses custom decorations to produce WKT text from spatial column and trying to use normal, nongeomodel as a originating model and .select_related() fails with exceptions.

Note:

In multidb branch this appears to be problem of sql compiler that expects that all related models uses same compiler as originating model. Which for example in GeoDjango models case is not true - they have their own compiler that adds spatial field decorations to queries causing different errors on different spatial backends.

by jbronn, 14 years ago

Attachment: select_related_fix_v1.diff added

comment:3 by Jani Tiainen, 14 years ago

Since multidb landed on trunk I update this ticket:

When using custom SQLCompiler that overrides get_columns code to mangle column selects instead of just getting fields as they are doesn't work with Django when using select_related() queryset method.

select_related() always uses it's own Manager SQLCompiler to construct SQL needed to query fields. (code that generates SQL for related fields is in django/trunk/django/db/models/sql/compiler.py around line 209). For example contributed gis application uses it's own thus creating it impossible to mix non-geo and geomodels.

Each related field should produce SQL using their own SQLCompiler instead of using originating model SQLCompiler which will produce incorrect SQL.

by jbronn, 14 years ago

Attachment: select_related_fix_v2.diff added

by Jani Tiainen, 14 years ago

Attachment: issue_12344.tar.gz added

Sample case when select_related() fails to use custom compiler

by anonymous, 14 years ago

comment:4 by jbronn, 14 years ago

Resolution: fixed
Status: newclosed

(In [12022]) Fixed #12344 -- Using select_related() on geographic fields with the Oracle spatial backend now works.

comment:5 by jbronn, 14 years ago

(In [12023]) [1.1.X] Fixed #12344 -- Using select_related() on geographic fields with the Oracle spatial backend now works.

Backport of r12022 from trunk.

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