Skip to content

Commit

Permalink
Fixed #16187 -- refactored ORM lookup system
Browse files Browse the repository at this point in the history
Allowed users to specify which lookups or transforms ("nested lookus")
are available for fields. The implementation is now class based.

Squashed commit of the following:

commit fa7a719
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 18 10:53:24 2014 +0200

    Added lookup registration API docs

commit eb1c8ce
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Tue Jan 14 18:59:36 2014 +0200

    Release notes and other minor docs changes

commit 11501c2
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Jan 12 20:53:03 2014 +0200

    Forgot to add custom_lookups tests in prev commit

commit 83173b9
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Jan 12 19:59:12 2014 +0200

    Renamed Extract -> Transform

commit 3b18d9f
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Jan 12 19:51:53 2014 +0200

    Removed suggestion of temporary lookup registration from docs

commit 21d0c76
Merge: 2509006 f2dc442
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Jan 12 09:38:23 2014 -0800

    Merge pull request #2 from mjtamlyn/lookups_3

    Reworked custom lookups docs.

commit f2dc442
Author: Marc Tamlyn <marc.tamlyn@gmail.com>
Date:   Sun Jan 12 13:15:05 2014 +0000

    Reworked custom lookups docs.

    Mostly just formatting and rewording, but also replaced the example
    using ``YearExtract`` to  use an example which is unlikely to ever be
    possible directly in the ORM.

commit 2509006
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Jan 12 13:19:13 2014 +0200

    Removed unused import

commit 4fba5df
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 22:34:41 2014 +0200

    Added docs to index

commit 6d53963
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 22:10:24 2014 +0200

    Dead code removal

commit f9cc039
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 19:00:43 2014 +0200

    A new try for docs

commit 33aa18a
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 14:57:12 2014 +0200

    Renamed get_cols to get_group_by_cols

commit c7d5f86
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 14:45:53 2014 +0200

    Altered query string customization for backends vendors

    The new way is trying to call first method 'as_' + connection.vendor.
    If that doesn't exist, then call as_sql().

    Also altered how lookup registration is done. There is now
    RegisterLookupMixin class that is used by Field, Extract and
    sql.Aggregate. This allows one to register lookups for extracts and
    aggregates in the same way lookup registration is done for fields.

commit 90e7004
Merge: 66649ff f7c2c0a
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 13:21:01 2014 +0200

    Merge branch 'master' into lookups_3

commit 66649ff
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Jan 11 13:16:01 2014 +0200

    Some rewording in docs

commit 31b8faa
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Dec 29 15:52:29 2013 +0200

    Cleanup based on review comments

commit 1016159
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Dec 28 18:37:04 2013 +0200

    Proof-of-concept fix for #16731

    Implemented only for SQLite and PostgreSQL, and only for startswith
    and istartswith lookups.

commit 193cd09
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Dec 28 17:57:58 2013 +0200

    Fixed #11722 -- iexact=F() produced invalid SQL

commit 08ed3c3
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Dec 21 23:59:52 2013 +0200

    Made Lookup and Extract available from django.db.models

commit b99c8d8
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Dec 21 23:06:29 2013 +0200

    Fixed review notes by Loic

commit 049eebc
Merge: ed8fab7 b80a835
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Dec 21 22:53:10 2013 +0200

    Merge branch 'master' into lookups_3

    Conflicts:
    	django/db/models/fields/__init__.py
    	django/db/models/sql/compiler.py
    	django/db/models/sql/query.py
    	tests/null_queries/tests.py

commit ed8fab7
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Dec 21 22:47:23 2013 +0200

    Made Extracts aware of full lookup path

commit 27a57b7
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Dec 1 21:10:11 2013 +0200

    Removed debugger import

commit 074e0f5
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Dec 1 21:02:16 2013 +0200

    GIS lookup support added

commit 760e28e
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Dec 1 20:04:31 2013 +0200

    Removed usage of Constraint, used Lookup instead

commit eac4776
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Dec 1 02:22:30 2013 +0200

    Minor cleanup of Lookup API

commit 2adf504
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sun Dec 1 02:14:19 2013 +0200

    Added documentation, polished implementation

commit 32c0435
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Nov 30 23:10:15 2013 +0200

    Avoid OrderedDict creation on lookup aggregate check

commit 7c8b3a3
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Sat Nov 30 23:04:34 2013 +0200

    Implemented nested lookups

    But there is no support of using lookups outside filtering yet.

commit 4d219d4
Author: Anssi Kääriäinen <akaariai@gmail.com>
Date:   Wed Nov 27 22:07:30 2013 +0200

    Initial implementation of custom lookups
  • Loading branch information
akaariai committed Jan 18, 2014
1 parent b87c59b commit 20bab2c
Show file tree
Hide file tree
Showing 38 changed files with 1,324 additions and 185 deletions.
5 changes: 3 additions & 2 deletions django/contrib/contenttypes/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from django.db.models import signals
from django.db.models.fields.related import ForeignObject, ForeignObjectRel
from django.db.models.related import PathInfo
from django.db.models.sql.where import Constraint
from django.db.models.sql.datastructures import Col
from django.forms import ModelForm, ALL_FIELDS
from django.forms.models import (BaseModelFormSet, modelformset_factory,
modelform_defines_fields)
Expand Down Expand Up @@ -236,7 +236,8 @@ def get_extra_restriction(self, where_class, alias, remote_alias):
field = self.rel.to._meta.get_field_by_name(self.content_type_field_name)[0]
contenttype_pk = self.get_content_type().pk
cond = where_class()
cond.add((Constraint(remote_alias, field.column, field), 'exact', contenttype_pk), 'AND')
lookup = field.get_lookup('exact')(Col(remote_alias, field, field), contenttype_pk)
cond.add(lookup, 'AND')
return cond

def bulk_related_objects(self, objs, using=DEFAULT_DB_ALIAS):
Expand Down
4 changes: 1 addition & 3 deletions django/contrib/gis/db/backends/mysql/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ def get_geom_placeholder(self, value, srid):
return placeholder

def spatial_lookup_sql(self, lvalue, lookup_type, value, field, qn):
alias, col, db_type = lvalue

geo_col = '%s.%s' % (qn(alias), qn(col))
geo_col, db_type = lvalue

lookup_info = self.geometry_functions.get(lookup_type, False)
if lookup_info:
Expand Down
5 changes: 1 addition & 4 deletions django/contrib/gis/db/backends/oracle/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,7 @@ def transform_value(val, srid):

def spatial_lookup_sql(self, lvalue, lookup_type, value, field, qn):
"Returns the SQL WHERE clause for use in Oracle spatial SQL construction."
alias, col, db_type = lvalue

# Getting the quoted table name as `geo_col`.
geo_col = '%s.%s' % (qn(alias), qn(col))
geo_col, db_type = lvalue

# See if a Oracle Geometry function matches the lookup type next
lookup_info = self.geometry_functions.get(lookup_type, False)
Expand Down
5 changes: 1 addition & 4 deletions django/contrib/gis/db/backends/postgis/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,10 +478,7 @@ def spatial_lookup_sql(self, lvalue, lookup_type, value, field, qn):
(alias, col, db_type), the lookup type string, lookup value, and
the geometry field.
"""
alias, col, db_type = lvalue

# Getting the quoted geometry column.
geo_col = '%s.%s' % (qn(alias), qn(col))
geo_col, db_type = lvalue

if lookup_type in self.geometry_operators:
if field.geography and not lookup_type in self.geography_operators:
Expand Down
5 changes: 1 addition & 4 deletions django/contrib/gis/db/backends/spatialite/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,7 @@ def spatial_lookup_sql(self, lvalue, lookup_type, value, field, qn):
[a tuple of (alias, column, db_type)], lookup type, lookup
value, the model field, and the quoting function.
"""
alias, col, db_type = lvalue

# Getting the quoted field as `geo_col`.
geo_col = '%s.%s' % (qn(alias), qn(col))
geo_col, db_type = lvalue

if lookup_type in self.geometry_functions:
# See if a SpatiaLite geometry function matches the lookup type.
Expand Down
15 changes: 15 additions & 0 deletions django/contrib/gis/db/models/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from django.db.models.sql.constants import QUERY_TERMS

GIS_LOOKUPS = {
'bbcontains', 'bboverlaps', 'contained', 'contains',
'contains_properly', 'coveredby', 'covers', 'crosses', 'disjoint',
'distance_gt', 'distance_gte', 'distance_lt', 'distance_lte',
'dwithin', 'equals', 'exact',
'intersects', 'overlaps', 'relate', 'same_as', 'touches', 'within',
'left', 'right', 'overlaps_left', 'overlaps_right',
'overlaps_above', 'overlaps_below',
'strictly_above', 'strictly_below'
}
ALL_TERMS = GIS_LOOKUPS | QUERY_TERMS

__all__ = ['ALL_TERMS', 'GIS_LOOKUPS']
6 changes: 6 additions & 0 deletions django/contrib/gis/db/models/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
from django.db.models.sql.expressions import SQLEvaluator
from django.utils.translation import ugettext_lazy as _
from django.contrib.gis import forms
from django.contrib.gis.db.models.constants import GIS_LOOKUPS
from django.contrib.gis.db.models.lookups import GISLookup
from django.contrib.gis.db.models.proxy import GeometryProxy
from django.contrib.gis.geometry.backend import Geometry, GeometryException
from django.utils import six
Expand Down Expand Up @@ -284,6 +286,10 @@ def get_placeholder(self, value, connection):
"""
return connection.ops.get_geom_placeholder(self, value)

for lookup_name in GIS_LOOKUPS:
lookup = type(lookup_name, (GISLookup,), {'lookup_name': lookup_name})
GeometryField.register_lookup(lookup)


# The OpenGIS Geometry Type Fields
class PointField(GeometryField):
Expand Down
28 changes: 28 additions & 0 deletions django/contrib/gis/db/models/lookups.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from django.db.models.lookups import Lookup
from django.db.models.sql.expressions import SQLEvaluator


class GISLookup(Lookup):
def as_sql(self, qn, connection):
from django.contrib.gis.db.models.sql import GeoWhereNode
# We use the same approach as was used by GeoWhereNode. It would
# be a good idea to upgrade GIS to use similar code that is used
# for other lookups.
if isinstance(self.rhs, SQLEvaluator):
# Make sure the F Expression destination field exists, and
# set an `srid` attribute with the same as that of the
# destination.
geo_fld = GeoWhereNode._check_geo_field(self.rhs.opts, self.rhs.expression.name)
if not geo_fld:
raise ValueError('No geographic field found in expression.')
self.rhs.srid = geo_fld.srid
db_type = self.lhs.output_type.db_type(connection=connection)
params = self.lhs.output_type.get_db_prep_lookup(
self.lookup_name, self.rhs, connection=connection)
lhs_sql, lhs_params = self.process_lhs(qn, connection)
# lhs_params not currently supported.
assert not lhs_params
data = (lhs_sql, db_type)
spatial_sql, spatial_params = connection.ops.spatial_lookup_sql(
data, self.lookup_name, self.rhs, self.lhs.output_type, qn)
return spatial_sql, spatial_params + params
14 changes: 1 addition & 13 deletions django/contrib/gis/db/models/sql/query.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.db import connections
from django.db.models.query import sql

from django.contrib.gis.db.models.constants import ALL_TERMS
from django.contrib.gis.db.models.fields import GeometryField
from django.contrib.gis.db.models.sql import aggregates as gis_aggregates
from django.contrib.gis.db.models.sql.conversion import AreaField, DistanceField, GeomField
Expand All @@ -9,19 +10,6 @@
from django.contrib.gis.measure import Area, Distance


ALL_TERMS = set([
'bbcontains', 'bboverlaps', 'contained', 'contains',
'contains_properly', 'coveredby', 'covers', 'crosses', 'disjoint',
'distance_gt', 'distance_gte', 'distance_lt', 'distance_lte',
'dwithin', 'equals', 'exact',
'intersects', 'overlaps', 'relate', 'same_as', 'touches', 'within',
'left', 'right', 'overlaps_left', 'overlaps_right',
'overlaps_above', 'overlaps_below',
'strictly_above', 'strictly_below'
])
ALL_TERMS.update(sql.constants.QUERY_TERMS)


class GeoQuery(sql.Query):
"""
A single spatial SQL query.
Expand Down
3 changes: 3 additions & 0 deletions django/db/backends/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,9 @@ class BaseDatabaseFeatures(object):
# What kind of error does the backend throw when accessing closed cursor?
closed_cursor_error_class = ProgrammingError

# Does 'a' LIKE 'A' match?
has_case_insensitive_like = True

def __init__(self, connection):
self.connection = connection

Expand Down
6 changes: 6 additions & 0 deletions django/db/backends/postgresql_psycopg2/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
supports_combined_alters = True
nulls_order_largest = True
closed_cursor_error_class = InterfaceError
has_case_insensitive_like = False


class DatabaseWrapper(BaseDatabaseWrapper):
Expand All @@ -83,6 +84,11 @@ class DatabaseWrapper(BaseDatabaseWrapper):
'iendswith': 'LIKE UPPER(%s)',
}

pattern_ops = {
'startswith': "LIKE %s || '%%%%'",
'istartswith': "LIKE UPPER(%s) || '%%%%'",
}

Database = Database

def __init__(self, *args, **kwargs):
Expand Down
5 changes: 5 additions & 0 deletions django/db/backends/sqlite3/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,11 @@ class DatabaseWrapper(BaseDatabaseWrapper):
'iendswith': "LIKE %s ESCAPE '\\'",
}

pattern_ops = {
'startswith': "LIKE %s || '%%%%'",
'istartswith': "LIKE UPPER(%s) || '%%%%'",
}

Database = Database

def __init__(self, *args, **kwargs):
Expand Down
1 change: 1 addition & 0 deletions django/db/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from django.db.models.fields.proxy import OrderWrt # NOQA
from django.db.models.deletion import ( # NOQA
CASCADE, PROTECT, SET, SET_NULL, SET_DEFAULT, DO_NOTHING, ProtectedError)
from django.db.models.lookups import Lookup, Transform # NOQA
from django.db.models import signals # NOQA


Expand Down
9 changes: 5 additions & 4 deletions django/db/models/aggregates.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ def refs_aggregate(lookup_parts, aggregates):
default annotation names we must check each prefix of the lookup_parts
for match.
"""
for i in range(len(lookup_parts) + 1):
if LOOKUP_SEP.join(lookup_parts[0:i]) in aggregates:
return True
return False
for n in range(len(lookup_parts) + 1):
level_n_lookup = LOOKUP_SEP.join(lookup_parts[0:n])
if level_n_lookup in aggregates:
return aggregates[level_n_lookup], lookup_parts[n:]
return False, ()


class Aggregate(object):
Expand Down
9 changes: 6 additions & 3 deletions django/db/models/fields/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from django.apps import apps
from django.db import connection
from django.db.models.lookups import default_lookups, RegisterLookupMixin
from django.db.models.query_utils import QueryWrapper
from django.conf import settings
from django import forms
Expand Down Expand Up @@ -80,7 +81,7 @@ def _empty(of_cls):


@total_ordering
class Field(object):
class Field(RegisterLookupMixin):
"""Base class for all field types"""

# Designates whether empty strings fundamentally are allowed at the
Expand All @@ -101,6 +102,7 @@ class Field(object):
'unique': _('%(model_name)s with this %(field_label)s '
'already exists.'),
}
class_lookups = default_lookups.copy()

# Generic field type description, usually overridden by subclasses
def _description(self):
Expand Down Expand Up @@ -514,8 +516,7 @@ def get_prep_lookup(self, lookup_type, value):
except ValueError:
raise ValueError("The __year lookup type requires an integer "
"argument")

raise TypeError("Field has invalid lookup: %s" % lookup_type)
return self.get_prep_value(value)

def get_db_prep_lookup(self, lookup_type, value, connection,
prepared=False):
Expand Down Expand Up @@ -564,6 +565,8 @@ def get_db_prep_lookup(self, lookup_type, value, connection,
return connection.ops.year_lookup_bounds_for_date_field(value)
else:
return [value] # this isn't supposed to happen
else:
return [value]

def has_default(self):
"""
Expand Down
36 changes: 22 additions & 14 deletions django/db/models/fields/related.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
from django.db.models import signals, Q
from django.db.models.fields import (AutoField, Field, IntegerField,
PositiveIntegerField, PositiveSmallIntegerField, FieldDoesNotExist)
from django.db.models.lookups import IsNull
from django.db.models.related import RelatedObject, PathInfo
from django.db.models.query import QuerySet
from django.db.models.deletion import CASCADE
from django.db.models.sql.datastructures import Col
from django.utils.encoding import smart_text
from django.utils import six
from django.utils.deprecation import RenameMethodsBase
Expand Down Expand Up @@ -987,6 +989,11 @@ def set_field_name(self):
# example custom multicolumn joins currently have no remote field).
self.field_name = None

def get_lookup_constraint(self, constraint_class, alias, targets, sources, lookup_type,
raw_value):
return self.field.get_lookup_constraint(constraint_class, alias, targets, sources,
lookup_type, raw_value)


class ManyToOneRel(ForeignObjectRel):
def __init__(self, field, to, field_name, related_name=None, limit_choices_to=None,
Expand Down Expand Up @@ -1193,14 +1200,16 @@ def get_reverse_path_info(self):
pathinfos = [PathInfo(from_opts, opts, (opts.pk,), self.rel, not self.unique, False)]
return pathinfos

def get_lookup_constraint(self, constraint_class, alias, targets, sources, lookup_type,
def get_lookup_constraint(self, constraint_class, alias, targets, sources, lookups,
raw_value):
from django.db.models.sql.where import SubqueryConstraint, Constraint, AND, OR
from django.db.models.sql.where import SubqueryConstraint, AND, OR
root_constraint = constraint_class()
assert len(targets) == len(sources)
if len(lookups) > 1:
raise exceptions.FieldError('Relation fields do not support nested lookups')
lookup_type = lookups[0]

def get_normalized_value(value):

from django.db.models import Model
if isinstance(value, Model):
value_list = []
Expand All @@ -1221,28 +1230,27 @@ def get_normalized_value(value):
[source.name for source in sources], raw_value),
AND)
elif lookup_type == 'isnull':
root_constraint.add(
(Constraint(alias, targets[0].column, targets[0]), lookup_type, raw_value), AND)
root_constraint.add(IsNull(Col(alias, targets[0], sources[0]), raw_value), AND)
elif (lookup_type == 'exact' or (lookup_type in ['gt', 'lt', 'gte', 'lte']
and not is_multicolumn)):
value = get_normalized_value(raw_value)
for index, source in enumerate(sources):
for target, source, val in zip(targets, sources, value):
lookup_class = target.get_lookup(lookup_type)
root_constraint.add(
(Constraint(alias, targets[index].column, sources[index]), lookup_type,
value[index]), AND)
lookup_class(Col(alias, target, source), val), AND)
elif lookup_type in ['range', 'in'] and not is_multicolumn:
values = [get_normalized_value(value) for value in raw_value]
value = [val[0] for val in values]
root_constraint.add(
(Constraint(alias, targets[0].column, sources[0]), lookup_type, value), AND)
lookup_class = targets[0].get_lookup(lookup_type)
root_constraint.add(lookup_class(Col(alias, targets[0], sources[0]), value), AND)
elif lookup_type == 'in':
values = [get_normalized_value(value) for value in raw_value]
for value in values:
value_constraint = constraint_class()
for index, target in enumerate(targets):
value_constraint.add(
(Constraint(alias, target.column, sources[index]), 'exact', value[index]),
AND)
for source, target, val in zip(sources, targets, value):
lookup_class = target.get_lookup('exact')
lookup = lookup_class(Col(alias, target, source), val)
value_constraint.add(lookup, AND)
root_constraint.add(value_constraint, OR)
else:
raise TypeError('Related Field got invalid lookup: %s' % lookup_type)
Expand Down
Loading

1 comment on commit 20bab2c

@jonashaag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks a lot for this feature.

Please sign in to comment.