Skip to content

Commit ef7130b

Browse files
ortholeviatortimabbott
authored andcommitted
query: process RHS according to its type + tests
`Field.get_db_prep_lookup` merely "prepares" the right hand side operand, and returns a (sql, params) pair. For a plain value `params` will simply be the value itself; compound expressions with `as_sql` method, however, are never designed to work with `get_db_prep_lookup` and instead needs their `as_sql` method called directly to retrieve the pair. The error message has been changed from time to time across Django versions. Related issues include: * #89 "can't adapt type 'CombinedExpression'" in admin filter Since Django 1.10 removed `Field.get_db_prep_lookup`, `Lookup.get_db_prep_lookup` implementation is simply as follows. ```python def get_db_prep_lookup(self, value, connection): return ('%s', [value]) ``` `CombinedExpression` is an expression type returned by combining exprssions with operators, such as `__add__` or `bitor`. They are never meant to be passed to the SQL engine directly; instead, it has an `as_sql` method that returns what we want. `process_rhs` first applies any bilateral transforms on both sides, and if it finds either `as_sql` or `_as_sql` method on the RHS, calls it and wraps the SQL in the pair with parentheses; otherwise, it diverts to the usual `get_db_prep_lookup` as above. * #64 'CombinedExpression' object is not iterable when using an admin filter on Django 1.8 Same as above, except `Lookup.get_db_prep_lookup` is as follows. ```python def get_db_prep_lookup(self, value, connection): return ( '%s', self.lhs.output_field.get_db_prep_lookup( self.lookup_name, value, connection, prepared=True)) ``` Following is the relevant part of `Field.get_db_prep_lookup`. ```python def get_db_prep_lookup(self, lookup_type, value, connection, prepared=False): """ Returns field's value prepared for database lookup. """ if not prepared: value = self.get_prep_lookup(lookup_type, value) prepared = True if hasattr(value, 'get_compiler'): value = value.get_compiler(connection=connection) if hasattr(value, 'as_sql') or hasattr(value, '_as_sql'): # If the value has a relabeled_clone method it means the # value will be handled later on. if hasattr(value, 'relabeled_clone'): return value if hasattr(value, 'as_sql'): sql, params = value.as_sql() else: sql, params = value._as_sql(connection=connection) return QueryWrapper(('(%s)' % sql), params) if lookup_type in ('month', 'day', 'week_day', 'hour', 'minute', 'second', 'search', 'regex', 'iregex', 'contains', 'icontains', 'iexact', 'startswith', 'endswith', 'istartswith', 'iendswith'): return [value] ``` Normally it is supposed to return the parameters for the SQL "prepared statement" as a `list`; however, if the value has either `as_sql` or `_as_sql`, it either returns the value directly if it also has `relabeled_clone` (which is the case for expressions) or wraps it in `QueryWrapper`. This is not a desired behavior, and using them would result in `TypeError` saying the object was not iterable since they were not lists in the first place. * #61 'SQLEvaluator' object is not iterable when using admin filter on Django 1.7 Same as above, except `SQLEvaluator` emerges from the SQL compiler wrapping expressions for internal use. - Update expressions: django.db.models.sql.compiler.SQLUpdateCompiler.as_sql: ... if hasattr(val, 'evaluate'): val = SQLEvaluator(val, self.query, allow_joins=False) ... - filter() resolution: django.db.models.sql.query.Query.add_filter: ... elif isinstance(value, ExpressionNode): # If value is a query expression, evaluate it value = SQLEvaluator(value, self, reuse=can_reuse) having_clause = value.contains_aggregate ... This commit also causes "regression" for the following issue; its legitimacy, however, is ambiguous considering that the lookup essentially "hijacks" the original definition of 'exact'. #26 admin BitFieldListFilter not working as expected It should be noted, however, that the original behavior is still possible to achieve by wrapping the `int` in a `BitHandler`. Fixing this correctly would involve treating `int` as `Bit` as before while showing a deprecation warning. Meanwhile `filter(flags=Value(4))` will find a record of which `flags` is exactly `4` across all versions.
1 parent 43f087d commit ef7130b

File tree

2 files changed

+38
-10
lines changed

2 files changed

+38
-10
lines changed

bitfield/query.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,28 @@
55

66

77
class BitQueryLookupWrapper(Exact): # NOQA
8-
def process_lhs(self, qn, connection, lhs=None):
9-
lhs_sql, params = super(BitQueryLookupWrapper, self).process_lhs(
10-
qn, connection, lhs)
11-
if self.rhs:
12-
lhs_sql = lhs_sql + ' & %s'
13-
else:
14-
lhs_sql = lhs_sql + ' | %s'
15-
params.extend(self.get_db_prep_lookup(self.rhs, connection)[1])
16-
return lhs_sql, params
8+
def process_lhs(self, compiler, connection, lhs=None):
9+
lhs_sql, lhs_params = super(BitQueryLookupWrapper, self).process_lhs(
10+
compiler, connection, lhs)
11+
12+
if not isinstance(self.rhs, (BitHandler, Bit)):
13+
return lhs_sql, lhs_params
14+
15+
op = ' & ' if self.rhs else ' | '
16+
rhs_sql, rhs_params = self.process_rhs(compiler, connection)
17+
params = list(lhs_params)
18+
params.extend(rhs_params)
19+
20+
return op.join((lhs_sql, rhs_sql)), params
1721

1822
def get_db_prep_lookup(self, value, connection):
1923
v = value.mask if isinstance(value, (BitHandler, Bit)) else value
2024
return super(BitQueryLookupWrapper, self).get_db_prep_lookup(v, connection)
2125

2226
def get_prep_lookup(self):
23-
return self.rhs
27+
if isinstance(self.rhs, (BitHandler, Bit)):
28+
return self.rhs # resolve at later stage, in get_db_prep_lookup
29+
return super(BitQueryLookupWrapper, self).get_prep_lookup()
2430

2531

2632
class BitQuerySaveWrapper(BitQueryLookupWrapper):

bitfield/tests/tests.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,28 @@ def test_select(self):
177177
self.assertFalse(BitFieldTestModel.objects.exclude(flags=BitFieldTestModel.flags.FLAG_0).exists())
178178
self.assertFalse(BitFieldTestModel.objects.exclude(flags=BitFieldTestModel.flags.FLAG_1).exists())
179179

180+
def test_select_complex_expression(self):
181+
BitFieldTestModel.objects.create(flags=3)
182+
self.assertTrue(BitFieldTestModel.objects.filter(flags=F('flags').bitor(BitFieldTestModel.flags.FLAG_1)).exists())
183+
self.assertTrue(BitFieldTestModel.objects.filter(flags=F('flags').bitor(BitFieldTestModel.flags.FLAG_0)).exists())
184+
self.assertTrue(BitFieldTestModel.objects.filter(flags=F('flags').bitor(BitFieldTestModel.flags.FLAG_0).bitor(BitFieldTestModel.flags.FLAG_1)).exists())
185+
self.assertTrue(BitFieldTestModel.objects.filter(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_0 | BitFieldTestModel.flags.FLAG_1)).exists())
186+
self.assertTrue(BitFieldTestModel.objects.filter(flags=F('flags').bitand(15)).exists())
187+
self.assertTrue(BitFieldTestModel.objects.exclude(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_2)).exists())
188+
self.assertTrue(BitFieldTestModel.objects.exclude(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_3)).exists())
189+
self.assertTrue(BitFieldTestModel.objects.exclude(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_2 | BitFieldTestModel.flags.FLAG_3)).exists())
190+
self.assertTrue(BitFieldTestModel.objects.exclude(flags=F('flags').bitand(12)).exists())
191+
192+
self.assertFalse(BitFieldTestModel.objects.exclude(flags=F('flags').bitor(BitFieldTestModel.flags.FLAG_1)).exists())
193+
self.assertFalse(BitFieldTestModel.objects.exclude(flags=F('flags').bitor(BitFieldTestModel.flags.FLAG_0)).exists())
194+
self.assertFalse(BitFieldTestModel.objects.exclude(flags=F('flags').bitor(BitFieldTestModel.flags.FLAG_0).bitor(BitFieldTestModel.flags.FLAG_1)).exists())
195+
self.assertFalse(BitFieldTestModel.objects.exclude(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_0 | BitFieldTestModel.flags.FLAG_1)).exists())
196+
self.assertFalse(BitFieldTestModel.objects.exclude(flags=F('flags').bitand(15)).exists())
197+
self.assertFalse(BitFieldTestModel.objects.filter(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_2)).exists())
198+
self.assertFalse(BitFieldTestModel.objects.filter(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_3)).exists())
199+
self.assertFalse(BitFieldTestModel.objects.filter(flags=F('flags').bitand(BitFieldTestModel.flags.FLAG_2 | BitFieldTestModel.flags.FLAG_3)).exists())
200+
self.assertFalse(BitFieldTestModel.objects.filter(flags=F('flags').bitand(12)).exists())
201+
180202
def test_update(self):
181203
instance = BitFieldTestModel.objects.create(flags=0)
182204
self.assertFalse(instance.flags.FLAG_0)

0 commit comments

Comments
 (0)