Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin BitFieldListFilter not working as expected #26

Open
bendavis78 opened this issue Nov 16, 2012 · 1 comment
Open

admin BitFieldListFilter not working as expected #26

bendavis78 opened this issue Nov 16, 2012 · 1 comment

Comments

@bendavis78
Copy link
Contributor

BitFieldListFilter seems to filter based on "exact", as opposed to a bitwise operation. So when you select the third item (mask=4), you essentially get this:

queryset.filter(flags=4)

Which will not return the desired result. The queryset() method should be overridden in order to perform the correct bitwise operation when filtering.

bendavis78 added a commit to bendavis78/django-bitfield that referenced this issue Nov 16, 2012
BitFieldListFilter now filters based on a "bitwise or" operation as opposed to
an exact match of the bitmask.
dcramer added a commit that referenced this issue Nov 26, 2012
tahajahangir added a commit to tahajahangir/django-bitfield that referenced this issue May 27, 2018
Reverting this commit actually fixes disqus#26
This reverts commit 769f032.
tahajahangir added a commit to tahajahangir/django-bitfield that referenced this issue Jan 10, 2019
Reverting this commit actually fixes disqus#26
This reverts commit 769f032.
ortholeviator added a commit to ortholeviator/django-bitfield that referenced this issue Apr 6, 2020
`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:

 * disqus#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.

 * disqus#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.

 * disqus#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'.

 disqus#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.
ortholeviator added a commit to ortholeviator/django-bitfield that referenced this issue Apr 6, 2020
`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:

 * disqus#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.

 * disqus#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.

 * disqus#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'.

 disqus#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.
ortholeviator added a commit to ortholeviator/django-bitfield that referenced this issue Apr 6, 2020
`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:

 * disqus#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.

 * disqus#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.

 * disqus#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'.

 disqus#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.
timabbott pushed a commit that referenced this issue Apr 6, 2020
`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.
@mateuszmandera
Copy link
Contributor

@ortholeviator What should be the status of this issue following #106?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants