From 219d928852c256a81d09dbaa29ed4cec42d2fdfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Sat, 1 Mar 2014 21:21:57 +0200 Subject: [PATCH] Fixed #21863 -- supplemented get_lookup() with get_transform() Also fixed #22124 -- Expanded explanation of exactly what is going on in as_sql() methods. --- django/db/models/lookups.py | 16 +++- django/db/models/sql/datastructures.py | 3 + django/db/models/sql/query.py | 29 +++---- docs/ref/models/custom-lookups.txt | 109 ++++++++++++++++++++----- tests/custom_lookups/tests.py | 56 ++++++++++++- tests/lookup/tests.py | 5 +- 6 files changed, 177 insertions(+), 41 deletions(-) diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 993f5a92ca..797f767a97 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -9,11 +9,11 @@ from django.utils.six.moves import xrange class RegisterLookupMixin(object): - def get_lookup(self, lookup_name): + def _get_lookup(self, lookup_name): try: return self.class_lookups[lookup_name] except KeyError: - # To allow for inheritance, check parent class class lookups. + # To allow for inheritance, check parent class' class_lookups. for parent in inspect.getmro(self.__class__): if not 'class_lookups' in parent.__dict__: continue @@ -26,6 +26,18 @@ class RegisterLookupMixin(object): return self.output_type.get_lookup(lookup_name) return None + def get_lookup(self, lookup_name): + found = self._get_lookup(lookup_name) + if found is not None and not issubclass(found, Lookup): + return None + return found + + def get_transform(self, lookup_name): + found = self._get_lookup(lookup_name) + if found is not None and not issubclass(found, Transform): + return None + return found + @classmethod def register_lookup(cls, lookup): if not 'class_lookups' in cls.__dict__: diff --git a/django/db/models/sql/datastructures.py b/django/db/models/sql/datastructures.py index 421c3cd860..634edf2477 100644 --- a/django/db/models/sql/datastructures.py +++ b/django/db/models/sql/datastructures.py @@ -24,6 +24,9 @@ class Col(object): def get_lookup(self, name): return self.output_type.get_lookup(name) + def get_transform(self, name): + return self.output_type.get_transform(name) + def prepare(self): return self diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 7bfdceb183..25ebe5598e 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1088,24 +1088,21 @@ class Query(object): lookups = lookups[:] while lookups: lookup = lookups[0] - next = lhs.get_lookup(lookup) + if len(lookups) == 1: + final_lookup = lhs.get_lookup(lookup) + if final_lookup: + return final_lookup(lhs, rhs) + # We didn't find a lookup, so we are going to try get_transform + # + get_lookup('exact'). + lookups.append('exact') + next = lhs.get_transform(lookup) if next: - if len(lookups) == 1: - # This was the last lookup, so return value lookup. - if issubclass(next, Transform): - lookups.append('exact') - lhs = next(lhs, lookups) - else: - return next(lhs, rhs) - else: - lhs = next(lhs, lookups) - # A field's get_lookup() can return None to opt for backwards - # compatibility path. - elif len(lookups) > 2: - raise FieldError( - "Unsupported lookup for field '%s'" % lhs.output_type.name) + lhs = next(lhs, lookups) else: - return None + raise FieldError( + "Unsupported lookup '%s' for %s or join on the field not " + "permitted." % + (lookup, lhs.output_type.__class__.__name__)) lookups = lookups[1:] def build_filter(self, filter_expr, branch_negated=False, current_negated=False, diff --git a/docs/ref/models/custom-lookups.txt b/docs/ref/models/custom-lookups.txt index 8aa2f792e5..2af69a3f82 100644 --- a/docs/ref/models/custom-lookups.txt +++ b/docs/ref/models/custom-lookups.txt @@ -60,6 +60,14 @@ and use ``NotEqual`` to generate the SQL. By convention, these names are always lowercase strings containing only letters, but the only hard requirement is that it must not contain the string ``__``. +We then need to define the ``as_sql`` method. This takes a ``SQLCompiler`` +object, called ``qn``, and the active database connection. ``SQLCompiler`` +objects are not documented, but the only thing we need to know about them is +that they have a ``compile()`` method which returns a tuple containing a SQL +string, and the parameters to be interpolated into that string. In most cases, +you don't need to use it directly and can pass it on to ``process_lhs()`` and +``process_rhs()``. + A ``Lookup`` works against two values, ``lhs`` and ``rhs``, standing for left-hand side and right-hand side. The left-hand side is usually a field reference, but it can be anything implementing the :ref:`query expression API @@ -69,11 +77,13 @@ reference to the ``name`` field of the ``Author`` model, and ``'Jack'`` is the right-hand side. We call ``process_lhs`` and ``process_rhs`` to convert them into the values we -need for SQL. In the above example, ``process_lhs`` returns -``('"author"."name"', [])`` and ``process_rhs`` returns ``('"%s"', ['Jack'])``. -In this example there were no parameters for the left hand side, but this would -depend on the object we have, so we still need to include them in the -parameters we return. +need for SQL using the ``qn`` object described before. These methods return +tuples containing some SQL and the parameters to be interpolated into that SQL, +just as we need to return from our ``as_sql`` method. In the above example, +``process_lhs`` returns ``('"author"."name"', [])`` and ``process_rhs`` returns +``('"%s"', ['Jack'])``. In this example there were no parameters for the left +hand side, but this would depend on the object we have, so we still need to +include them in the parameters we return. Finally we combine the parts into a SQL expression with ``<>``, and supply all the parameters for the query. We then return a tuple containing the generated @@ -216,6 +226,52 @@ When compiling a query, Django first looks for ``as_%s % connection.vendor`` methods, and then falls back to ``as_sql``. The vendor names for the in-built backends are ``sqlite``, ``postgresql``, ``oracle`` and ``mysql``. +How Django determines the lookups and transforms which are used +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In some cases you may which to dynamically change which ``Transform`` or +``Lookup`` is returned based on the name passed in, rather than fixing it. As +an example, you could have a field which stores coordinates or an arbitrary +dimension, and wish to allow a syntax like ``.filter(coords__x7=4)`` to return +the objects where the 7th coordinate has value 4. In order to do this, you +would override ``get_lookup`` with something like:: + + class CoordinatesField(Field): + def get_lookup(self, lookup_name): + if lookup_name.startswith('x'): + try: + dimension = int(lookup_name[1:]) + except ValueError: + pass + finally: + return get_coordinate_lookup(dimension) + return super(CoordinatesField, self).get_lookup(lookup_name) + +You would then define ``get_coordinate_lookup`` appropriately to return a +``Lookup`` subclass which handles the relevant value of ``dimension``. + +There is a similarly named method called ``get_transform()``. ``get_lookup()`` +should always return a ``Lookup`` subclass, and ``get_transform()`` a +``Transform`` subclass. It is important to remember that ``Transform`` +objects can be further filtered on, and ``Lookup`` objects cannot. + +When filtering, if there is only one lookup name remaining to be resolved, we +will look for a ``Lookup``. If there are multiple names, it will look for a +``Transform``. In the situation where there is only one name and a ``Lookup`` +is not found, we look for a ``Transform`` and then the ``exact`` lookup on that +``Transform``. All call sequences always end with a ``Lookup``. To clarify: + +- ``.filter(myfield__mylookup)`` will call ``myfield.get_lookup('mylookup')``. +- ``.filter(myfield__mytransform__mylookup)`` will call + ``myfield.get_transform('mytransform')``, and then + ``mytransform.get_lookup('mylookup')``. +- ``.filter(myfield__mytransform)`` will first call + ``myfield.get_lookup('mytransform')``, which will fail, so it will fall back + to calling ``myfield.get_transform('mytransform')`` and then + ``mytransform.get_lookup('exact')``. + +Lookups and transforms are registered using the same API - ``register_lookup``. + .. _query-expression: The Query Expression API @@ -228,21 +284,14 @@ to this API. .. method:: as_sql(qn, connection) Responsible for producing the query string and parameters for the - expression. The ``qn`` has a ``compile()`` method that can be used to - compile other expressions. The ``connection`` is the connection used to - execute the query. + expression. The ``qn`` is a ``SQLCompiler`` object, which has a + ``compile()`` method that can be used to compile other expressions. The + ``connection`` is the connection used to execute the query. Calling expression.as_sql() directly is usually incorrect - instead ``qn.compile(expression)`` should be used. The ``qn.compile()`` method will take care of calling vendor-specific methods of the expression. -.. method:: get_lookup(lookup_name) - - The ``get_lookup()`` method is used to fetch lookups. By default the - lookup is fetched from the expression's output type in the same way - described in registering and fetching lookup documentation below. - It is possible to override this method to alter that behavior. - .. method:: as_vendorname(qn, connection) Works like ``as_sql()`` method. When an expression is compiled by @@ -251,6 +300,21 @@ to this API. The vendorname is one of ``postgresql``, ``oracle``, ``sqlite`` or ``mysql`` for Django's built-in backends. +.. method:: get_lookup(lookup_name) + + The ``get_lookup()`` method is used to fetch lookups. By default the + lookup is fetched from the expression's output type in the same way + described in registering and fetching lookup documentation below. + It is possible to override this method to alter that behavior. + +.. method:: get_transform(lookup_name) + + The ``get_transform()`` method is used when a transform is needed rather + than a lookup, or if a lookup is not found. This is a more complex + situation which is useful when there arbitrary possible lookups for a + field. Generally speaking, you will not need to override ``get_lookup()`` + or ``get_transform()``, and can use ``register_lookup()`` instead. + .. attribute:: output_type The ``output_type`` attribute is used by the ``get_lookup()`` method to check for @@ -325,12 +389,19 @@ The lookup registration API is explained below. Registers the Lookup or Transform for the class. For example ``DateField.register_lookup(YearExact)`` will register ``YearExact`` for all ``DateFields`` in the project, but also for fields that are instances - of a subclass of ``DateField`` (for example ``DateTimeField``). + of a subclass of ``DateField`` (for example ``DateTimeField``). You can + register a Lookup or a Transform using the same class method. .. method:: get_lookup(lookup_name) - Django uses ``get_lookup(lookup_name)`` to fetch lookups or transforms. - The implementation of ``get_lookup()`` fetches lookups or transforms - registered for the current class based on their lookup_name attribute. + Django uses ``get_lookup(lookup_name)`` to fetch lookups. The + implementation of ``get_lookup()`` looks for a subclass which is registered + for the current class with the correct ``lookup_name``. + +.. method:: get_transform(lookup_name) + + Django uses ``get_transform(lookup_name)`` to fetch lookups. The + implementation of ``get_transform()`` looks for a subclass which is registered + for the current class with the correct ``transform_name``. The lookup registration API is available for ``Transform`` and ``Field`` classes. diff --git a/tests/custom_lookups/tests.py b/tests/custom_lookups/tests.py index 0ac1780796..396974b4b1 100644 --- a/tests/custom_lookups/tests.py +++ b/tests/custom_lookups/tests.py @@ -3,10 +3,11 @@ from __future__ import unicode_literals from datetime import date import unittest -from django.test import TestCase -from .models import Author +from django.core.exceptions import FieldError from django.db import models from django.db import connection +from django.test import TestCase +from .models import Author class Div3Lookup(models.Lookup): @@ -289,3 +290,54 @@ class YearLteTests(TestCase): finally: YearTransform._unregister_lookup(CustomYearExact) YearTransform.register_lookup(YearExact) + + +class TrackCallsYearTransform(YearTransform): + lookup_name = 'year' + call_order = [] + + def as_sql(self, qn, connection): + lhs_sql, params = qn.compile(self.lhs) + return connection.ops.date_extract_sql('year', lhs_sql), params + + @property + def output_type(self): + return models.IntegerField() + + def get_lookup(self, lookup_name): + self.call_order.append('lookup') + return super(TrackCallsYearTransform, self).get_lookup(lookup_name) + + def get_transform(self, lookup_name): + self.call_order.append('transform') + return super(TrackCallsYearTransform, self).get_transform(lookup_name) + + +class LookupTransformCallOrderTests(TestCase): + def test_call_order(self): + models.DateField.register_lookup(TrackCallsYearTransform) + try: + # junk lookup - tries lookup, then transform, then fails + with self.assertRaises(FieldError): + Author.objects.filter(birthdate__year__junk=2012) + self.assertEqual(TrackCallsYearTransform.call_order, + ['lookup', 'transform']) + TrackCallsYearTransform.call_order = [] + # junk transform - tries transform only, then fails + with self.assertRaises(FieldError): + Author.objects.filter(birthdate__year__junk__more_junk=2012) + self.assertEqual(TrackCallsYearTransform.call_order, + ['transform']) + TrackCallsYearTransform.call_order = [] + # Just getting the year (implied __exact) - lookup only + Author.objects.filter(birthdate__year=2012) + self.assertEqual(TrackCallsYearTransform.call_order, + ['lookup']) + TrackCallsYearTransform.call_order = [] + # Just getting the year (explicit __exact) - lookup only + Author.objects.filter(birthdate__year__exact=2012) + self.assertEqual(TrackCallsYearTransform.call_order, + ['lookup']) + + finally: + models.DateField._unregister_lookup(TrackCallsYearTransform) diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index df6192ee11..8a9b69b879 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -476,8 +476,9 @@ class LookupTests(TestCase): Article.objects.filter(headline__starts='Article') self.fail('FieldError not raised') except FieldError as ex: - self.assertEqual(str(ex), "Join on field 'headline' not permitted. " - "Did you misspell 'starts' for the lookup type?") + self.assertEqual( + str(ex), "Unsupported lookup 'starts' for CharField " + "or join on the field not permitted.") def test_regex(self): # Create some articles with a bit more interesting headlines for testing field lookups: