[Django] #29955: Support dwithin lookup on F expressions for distance stored in model

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
               Reporter:  Peter Bex  |          Owner:  nobody
                   Type:  New        |         Status:  new
  feature                            |
              Component:  GIS        |        Version:  2.1
               Severity:  Normal     |       Keywords:  distance, query
           Triage Stage:             |  builder, F expressions
  Unreviewed                         |      Has patch:  0
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 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')))
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29955>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/050.45e2fe14950890abd679deaadd28d5d5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/29955#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.0eb51b1a272f7a85dd8d7f3f80155f51%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
In reply to this post by Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Sergey Fedoseev):

 * cc: Sergey Fedoseev (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/29955#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.0e11651ad08f6b48401004c09c39589e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
In reply to this post by Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

 I suspect this has been fixed by 8a281aa7fe76a9da2284f943964a9413697cff1f
 (#30687) but I cannot confirm it because the provided test case doesn't
 work out of the fox (`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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29955#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.c8918479a91dd80ae2c615ed25d8f256%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
In reply to this post by Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by felixxm):

 * Attachment "test-29955.diff" added.


--
Ticket URL: <https://code.djangoproject.com/ticket/29955>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.bf6c2d47fe8a3d6649826196d789c31c%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
In reply to this post by Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by felixxm):

 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
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29955#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.a6c4de52a0657abf6cddd23fa9d63d13%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
In reply to this post by Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  Simon
                                     |  Charette
         Type:  New feature          |                   Status:  assigned
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * owner:  nobody => Simon Charette
 * status:  new => assigned
 * has_patch:  0 => 1


Comment:

 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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29955#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.d7dc6fbd41dfd9b39c7d25e8a87b2c8a%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29955: Support dwithin lookup on F expressions for distance stored in model

Django
In reply to this post by Django
#29955: Support dwithin lookup on F expressions for distance stored in model
-------------------------------------+-------------------------------------
     Reporter:  Peter Bex            |                    Owner:  Simon
                                     |  Charette
         Type:  New feature          |                   Status:  closed
    Component:  GIS                  |                  Version:  2.1
     Severity:  Normal               |               Resolution:  fixed
     Keywords:  distance, query      |             Triage Stage:  Accepted
  builder, F expressions             |
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"bb9e82f2748ace292a584841ab9af8696df27f53" bb9e82f2]:
 {{{
 #!CommitTicketReference repository=""
 revision="bb9e82f2748ace292a584841ab9af8696df27f53"
 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.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29955#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.854c6dbe3a7080e8bc9045902fac8067%40djangoproject.com.