Opened 5 years ago

Closed 5 years ago

#29955 closed New feature (fixed)

Support dwithin lookup on F expressions for distance stored in model

Reported by: Peter Bex Owned by: Simon Charette
Component: GIS Version: 2.1
Severity: Normal Keywords: distance, query builder, F expressions
Cc: Sergey Fedoseev Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Today I had to find all points of interest which occur within distance of a trajectory LineString, where each point of interest object stores how far away it should be from the trajectory in order to be included (we use it for determining which fuel stations are allowed for refueling).

I wanted to write the "obvious" thing like this:

import django.db.models import Model, TextField, F
from django.contrib.gis.db.models import PointField
from django.contrib.gis.geos import LineString

class PointOfInterest(Model):
    name = TextField()
    location = PointField(geography=True)

PointOfInterest.objects.filter(location__dwithin=(LineString(....), F('allowed_distance')))

Unfortunately this gives an error. The only thing that's allowed as the second object in the dwithin lookup is a Distance() object.

I tried to get around this but couldn't figure out exactly how the internals are supposed to work. With my limited understanding of the Django query internals, I came up with this custom hack (which you'll probably find godawful):

# Hack because __dwithin does not accept fields, only fixed Distance()
# values.
@BaseSpatialField.register_lookup
class DWithinFieldLookup(DistanceLookupBase):
	lookup_name = 'dwithin_field'
	sql_template = '%(func)s(%(lhs)s, %(rhs)s)'

	def get_rhs_op(self, connection, rhs):
		return connection.ops.gis_operators['dwithin']

	# Taken from django.db.models.lookups (super-superclass)
	def get_db_prep_lookup(self, value, connection):
		if isinstance(value, GEOSGeometry):
			return super().get_db_prep_lookup(value, connection)
		else:
			return ('%s', [value])

	# Taken from django.db.models.lookups (super-superclass)
	def process_rhs(self, compiler, connection):
		value1 = self.rhs
		value2 = self.rhs_params[0]
		if self.bilateral_transforms:
			if self.rhs_is_direct_value():
				# Do not call get_db_prep_lookup here as the value will be
				# transformed before being used for lookup
				value1 = Value(value1, output_field=self.lhs.output_field)
			value1 = self.apply_bilateral_transforms(value1)
			value1 = value1.resolve_expression(compiler.query)

		if self.bilateral_transforms:
			if self.rhs_is_direct_value():
				# Do not call get_db_prep_lookup here as the value will be
				# transformed before being used for lookup
				value2 = Value(value2, output_field=self.lhs.output_field)
			value2 = self.apply_bilateral_transforms(value2)
			value2 = value2.resolve_expression(compiler.query)

		if hasattr(value1, 'as_sql'):
			sql1, params1 = compiler.compile(value1)
			sql1 = '(' + sql1 + ')'
		else:
			sql1, params1 = self.get_db_prep_lookup(value1, connection)

		if hasattr(value2, 'resolve_expression'):
			value2 = value2.resolve_expression(query=compiler.query, allow_joins=True, reuse=None, summarize=False, for_save=False)
			sql2, params2 = compiler.compile(value2)
		elif hasattr(value2, 'as_sql'):
			sql2, params2 = compiler.compile(value2)
			sql2 = '(' + sql2 + ')'
		else:
			sql2, params2 = self.get_db_prep_lookup(value2, connection)

		return sql1 + ',' + sql2, params1 + params2

This allows me to express exactly as I originally wanted to:

PointOfInterest.objects.filter(location__dwithin_field=(LineString(....), F('allowed_distance')))

Attachments (1)

test-29955.diff (1.4 KB ) - added by Mariusz Felisiak 5 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 by Tim Graham, 5 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Sergey Fedoseev, 5 years ago

Cc: Sergey Fedoseev added

comment:3 by Simon Charette, 5 years ago

I suspect this has been fixed by 8a281aa7fe76a9da2284f943964a9413697cff1f (#30687) but I cannot confirm it because the provided test case doesn't work (allowed_distance is referring to a non existing field).

Peter, please confirm whether or not the aforementioned commit resolves your issue or provide a working test case or at least a full traceback.

Last edited 5 years ago by Simon Charette (previous) (diff)

by Mariusz Felisiak, 5 years ago

Attachment: test-29955.diff added

comment:4 by Mariusz Felisiak, 5 years ago

Simon it still doesn't work. I attached test case that raises

File "django/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
IndexError: tuple index out of range

comment:5 by Simon Charette, 5 years ago

Has patch: set
Owner: changed from nobody to Simon Charette
Status: newassigned

Thanks for the test case Mariusz, here's the patch.

https://github.com/django/django/pull/11681

This was missed when adding support for distance expression #25499 most probably because of the sql_template override.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In bb9e82f2:

Fixed #29955 -- Added support for distance expression to the dwithin lookup.

This was missed when adding support to other distance lookups in
refs #25499.

Thanks Peter Bex for the report and Mariusz for testcases.

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