Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #17728 -- When filtering an annotation, ensured the values used…

… in the filter are properly converted to their database representation. This bug was particularly visible with timezone-aware DateTimeFields. Thanks gg for the report and Carl for the review.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@17576 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 8b5361619818a8e16562d054d7dc8ddf61147384 1 parent c870318
Aymeric Augustin aaugustin authored
15 django/db/models/sql/aggregates.py
View
@@ -2,18 +2,11 @@
Classes to represent the default SQL aggregate functions
"""
-class AggregateField(object):
- """An internal field mockup used to identify aggregates in the
- data-conversion parts of the database backend.
- """
- def __init__(self, internal_type):
- self.internal_type = internal_type
-
- def get_internal_type(self):
- return self.internal_type
+from django.db.models.fields import IntegerField, FloatField
-ordinal_aggregate_field = AggregateField('IntegerField')
-computed_aggregate_field = AggregateField('FloatField')
+# Fake fields used to identify aggregate types in data-conversion operations.
+ordinal_aggregate_field = IntegerField()
+computed_aggregate_field = FloatField()
class Aggregate(object):
"""
46 django/db/models/sql/where.py
View
@@ -10,6 +10,7 @@
from django.utils import tree
from django.db.models.fields import Field
from django.db.models.sql.datastructures import EmptyResultSet, FullResultSet
+from django.db.models.sql.aggregates import Aggregate
# Connection types
AND = 'AND'
@@ -30,9 +31,8 @@ class WhereNode(tree.Node):
the correct SQL).
The children in this tree are usually either Q-like objects or lists of
- [table_alias, field_name, db_type, lookup_type, value_annotation,
- params]. However, a child could also be any class with as_sql() and
- relabel_aliases() methods.
+ [table_alias, field_name, db_type, lookup_type, value_annotation, params].
+ However, a child could also be any class with as_sql() and relabel_aliases() methods.
"""
default = AND
@@ -54,25 +54,22 @@ def add(self, data, connector):
# emptiness and transform any non-empty values correctly.
value = list(value)
- # The "annotation" parameter is used to pass auxilliary information
+ # The "value_annotation" parameter is used to pass auxilliary information
# about the value(s) to the query construction. Specifically, datetime
# and empty values need special handling. Other types could be used
# here in the future (using Python types is suggested for consistency).
if isinstance(value, datetime.datetime):
- annotation = datetime.datetime
+ value_annotation = datetime.datetime
elif hasattr(value, 'value_annotation'):
- annotation = value.value_annotation
+ value_annotation = value.value_annotation
else:
- annotation = bool(value)
+ value_annotation = bool(value)
if hasattr(obj, "prepare"):
value = obj.prepare(lookup_type, value)
- super(WhereNode, self).add((obj, lookup_type, annotation, value),
- connector)
- return
- super(WhereNode, self).add((obj, lookup_type, annotation, value),
- connector)
+ super(WhereNode, self).add(
+ (obj, lookup_type, value_annotation, value), connector)
def as_sql(self, qn, connection):
"""
@@ -132,21 +129,26 @@ def as_sql(self, qn, connection):
def make_atom(self, child, qn, connection):
"""
- Turn a tuple (table_alias, column_name, db_type, lookup_type,
- value_annot, params) into valid SQL.
+ Turn a tuple (Constraint(table_alias, column_name, db_type),
+ lookup_type, value_annotation, params) into valid SQL.
+
+ The first item of the tuple may also be an Aggregate.
Returns the string for the SQL fragment and the parameters to use for
it.
"""
- lvalue, lookup_type, value_annot, params_or_value = child
- if hasattr(lvalue, 'process'):
+ lvalue, lookup_type, value_annotation, params_or_value = child
+ if isinstance(lvalue, Constraint):
try:
lvalue, params = lvalue.process(lookup_type, params_or_value, connection)
except EmptyShortCircuit:
raise EmptyResultSet
+ elif isinstance(lvalue, Aggregate):
+ params = lvalue.field.get_db_prep_lookup(lookup_type, params_or_value, connection)
else:
- params = Field().get_db_prep_lookup(lookup_type, params_or_value,
- connection=connection, prepared=True)
+ raise TypeError("'make_atom' expects a Constraint or an Aggregate "
+ "as the first item of its 'child' argument.")
+
if isinstance(lvalue, tuple):
# A direct database column lookup.
field_sql = self.sql_for_columns(lvalue, qn, connection)
@@ -154,7 +156,7 @@ def make_atom(self, child, qn, connection):
# A smart object with an as_sql() method.
field_sql = lvalue.as_sql(qn, connection)
- if value_annot is datetime.datetime:
+ if value_annotation is datetime.datetime:
cast_sql = connection.ops.datetime_cast_sql()
else:
cast_sql = '%s'
@@ -168,7 +170,7 @@ def make_atom(self, child, qn, connection):
if (len(params) == 1 and params[0] == '' and lookup_type == 'exact'
and connection.features.interprets_empty_strings_as_nulls):
lookup_type = 'isnull'
- value_annot = True
+ value_annotation = True
if lookup_type in connection.operators:
format = "%s %%s %%s" % (connection.ops.lookup_cast(lookup_type),)
@@ -177,7 +179,7 @@ def make_atom(self, child, qn, connection):
extra), params)
if lookup_type == 'in':
- if not value_annot:
+ if not value_annotation:
raise EmptyResultSet
if extra:
return ('%s IN %s' % (field_sql, extra), params)
@@ -206,7 +208,7 @@ def make_atom(self, child, qn, connection):
params)
elif lookup_type == 'isnull':
return ('%s IS %sNULL' % (field_sql,
- (not value_annot and 'NOT ' or '')), ())
+ (not value_annotation and 'NOT ' or '')), ())
elif lookup_type == 'search':
return (connection.ops.fulltext_search_sql(field_sql), params)
elif lookup_type in ('regex', 'iregex'):
7 tests/modeltests/timezones/models.py
View
@@ -6,6 +6,13 @@ class Event(models.Model):
class MaybeEvent(models.Model):
dt = models.DateTimeField(blank=True, null=True)
+class Session(models.Model):
+ name = models.CharField(max_length=20)
+
+class SessionEvent(models.Model):
+ dt = models.DateTimeField()
+ session = models.ForeignKey(Session, related_name='events')
+
class Timestamp(models.Model):
created = models.DateTimeField(auto_now_add=True)
updated = models.DateTimeField(auto_now=True)
46 tests/modeltests/timezones/tests.py
View
@@ -25,7 +25,7 @@
from django.utils.unittest import skipIf, skipUnless
from .forms import EventForm, EventSplitForm, EventModelForm
-from .models import Event, MaybeEvent, Timestamp
+from .models import Event, MaybeEvent, Session, SessionEvent, Timestamp
# These tests use the EAT (Eastern Africa Time) and ICT (Indochina Time)
@@ -231,6 +231,28 @@ def test_query_aggregation(self):
'dt__max': datetime.datetime(2011, 9, 1, 23, 20, 20),
})
+ def test_query_annotation(self):
+ # Only min and max make sense for datetimes.
+ morning = Session.objects.create(name='morning')
+ afternoon = Session.objects.create(name='afternoon')
+ SessionEvent.objects.create(dt=datetime.datetime(2011, 9, 1, 23, 20, 20), session=afternoon)
+ SessionEvent.objects.create(dt=datetime.datetime(2011, 9, 1, 13, 20, 30), session=afternoon)
+ SessionEvent.objects.create(dt=datetime.datetime(2011, 9, 1, 3, 20, 40), session=morning)
+ morning_min_dt = datetime.datetime(2011, 9, 1, 3, 20, 40)
+ afternoon_min_dt = datetime.datetime(2011, 9, 1, 13, 20, 30)
+ self.assertQuerysetEqual(
+ Session.objects.annotate(dt=Min('events__dt')).order_by('dt'),
+ [morning_min_dt, afternoon_min_dt],
+ transform=lambda d: d.dt)
+ self.assertQuerysetEqual(
+ Session.objects.annotate(dt=Min('events__dt')).filter(dt__lt=afternoon_min_dt),
+ [morning_min_dt],
+ transform=lambda d: d.dt)
+ self.assertQuerysetEqual(
+ Session.objects.annotate(dt=Min('events__dt')).filter(dt__gte=afternoon_min_dt),
+ [afternoon_min_dt],
+ transform=lambda d: d.dt)
+
def test_query_dates(self):
Event.objects.create(dt=datetime.datetime(2011, 1, 1, 1, 30, 0))
Event.objects.create(dt=datetime.datetime(2011, 1, 1, 4, 30, 0))
@@ -412,6 +434,28 @@ def test_query_aggregation(self):
'dt__max': datetime.datetime(2011, 9, 1, 23, 20, 20, tzinfo=EAT),
})
+ def test_query_annotation(self):
+ # Only min and max make sense for datetimes.
+ morning = Session.objects.create(name='morning')
+ afternoon = Session.objects.create(name='afternoon')
+ SessionEvent.objects.create(dt=datetime.datetime(2011, 9, 1, 23, 20, 20, tzinfo=EAT), session=afternoon)
+ SessionEvent.objects.create(dt=datetime.datetime(2011, 9, 1, 13, 20, 30, tzinfo=EAT), session=afternoon)
+ SessionEvent.objects.create(dt=datetime.datetime(2011, 9, 1, 3, 20, 40, tzinfo=EAT), session=morning)
+ morning_min_dt = datetime.datetime(2011, 9, 1, 3, 20, 40, tzinfo=EAT)
+ afternoon_min_dt = datetime.datetime(2011, 9, 1, 13, 20, 30, tzinfo=EAT)
+ self.assertQuerysetEqual(
+ Session.objects.annotate(dt=Min('events__dt')).order_by('dt'),
+ [morning_min_dt, afternoon_min_dt],
+ transform=lambda d: d.dt)
+ self.assertQuerysetEqual(
+ Session.objects.annotate(dt=Min('events__dt')).filter(dt__lt=afternoon_min_dt),
+ [morning_min_dt],
+ transform=lambda d: d.dt)
+ self.assertQuerysetEqual(
+ Session.objects.annotate(dt=Min('events__dt')).filter(dt__gte=afternoon_min_dt),
+ [afternoon_min_dt],
+ transform=lambda d: d.dt)
+
def test_query_dates(self):
# Same comment as in test_query_date_related_filters.
Event.objects.create(dt=datetime.datetime(2011, 1, 1, 1, 30, 0, tzinfo=EAT))
Please sign in to comment.
Something went wrong with that request. Please try again.