Opened 9 years ago

Closed 9 years ago

#25150 closed Bug (wontfix)

Iterable objects are converted to lists during SQL handling

Reported by: Eldon Koyle Owned by: nobody
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords:
Cc: jay.mcentire@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have an iterable object (netaddr.IPNetwork) that I am trying to use as part of a query. The Django 1.8 update has introduced a regression in which the object is cast to a list; however, although the object is iterable we do not want to iterate through it but do a comparison on the object proper.

I have a patch which restores the desired behavior, but I'm not certain if there are any other side effects.

Attachments (2)

fix_django_iterator.patch (762 bytes ) - added by Eldon Koyle 9 years ago.
25150.diff (2.6 KB ) - added by Tim Graham 9 years ago.

Download all attachments as: .zip

Change History (7)

by Eldon Koyle, 9 years ago

Attachment: fix_django_iterator.patch added

comment:1 by Shai Berger, 9 years ago

Needs tests: set
Patch needs improvement: set

Hi,

Your description does not make it clear where you see the changed behavior. Your patch doesn't either. Also, as far as I understand, even with your patch, the iterator is consumed.

Can you provide a test-case which fails with current Django 1.8 and succeeds with your patch?

comment:2 by Tim Graham, 9 years ago

The patch causes 3 test failures: two in queries and one in lookups because, as Shai noted, the iterator is now consumed.

comment:3 by Eldon Koyle, 9 years ago

Sorry for the lack of information in my original report.

The problem is that my object, although it can be used as an iterator, shouldn't be used as such. It is a netaddr.IPNetwork object, and it can act as an iterator (it iterates over addresses in the network); however, it represents a network and not a list of addresses.

When cast to a list, we have problems around line 1200:

        else:
            col = targets[0].get_col(alias, field)
            condition = self.build_lookup(lookups, col, value)  # Traceback here, b/c a list is not valid for the lookup we are doing
            lookup_type = condition.lookup_name

However we don't want to iterate over the object at all (it will cause some seriously strange side effects to replace a network with a list of addresses).

We are using a django extension to handle postgresql INET and CIDR types (django-netfields which we have updated to work with django 1.8), but the problem is happening outside the netfields code (it is cast to a list in django/db/models/sql/query.py):

Object in question:

from netfields import CidrAddressField

class Network(models.Model):
    network = CidrAddressField(primary_key=True)

Example of failing code:

In [1]: import netaddr

In [2]: net = netaddr.IPNetwork('192.168.0.0/16')

In [3]: Network.objects.filter(network=net)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-961aa006d914> in <module>()
----> 1 Network.objects.filter(network=net)

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/manager.pyc in manager_method(self, *args, **kwargs)
    125         def create_method(name, method):
    126             def manager_method(self, *args, **kwargs):
--> 127                 return getattr(self.get_queryset(), name)(*args, **kwargs)
    128             manager_method.__name__ = method.__name__
    129             manager_method.__doc__ = method.__doc__

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/query.pyc in filter(self, *args, **kwargs)
    677         set.
    678         """
--> 679         return self._filter_or_exclude(False, *args, **kwargs)
    680 
    681     def exclude(self, *args, **kwargs):

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/query.pyc in _filter_or_exclude(self, negate, *args, **kwargs)
    695             clone.query.add_q(~Q(*args, **kwargs))
    696         else:
--> 697             clone.query.add_q(Q(*args, **kwargs))
    698         return clone
    699 

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/sql/query.pyc in add_q(self, q_object)
   1302         existing_inner = set(
   1303             (a for a in self.alias_map if self.alias_map[a].join_type == INNER))
-> 1304         clause, require_inner = self._add_q(where_part, self.used_aliases)
   1305         self.where.add(clause, AND)
   1306         for hp in having_parts:

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/sql/query.pyc in _add_q(self, q_object, used_aliases, branch_negated, current_negated, allow_joins, split_subq)
   1330                     child, can_reuse=used_aliases, branch_negated=branch_negated,
   1331                     current_negated=current_negated, connector=connector,
-> 1332                     allow_joins=allow_joins, split_subq=split_subq,
   1333                 )
   1334                 joinpromoter.add_votes(needed_inner)

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/sql/query.pyc in build_filter(self, filter_expr, branch_negated, current_negated, can_reuse, connector, allow_joins, split_subq)
   1201             else:
   1202                 col = targets[0].get_col(alias, field)
-> 1203             condition = self.build_lookup(lookups, col, value)
   1204             if not condition:
   1205                 # Backwards compat for custom lookups

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/sql/query.pyc in build_lookup(self, lookups, lhs, rhs)
   1094                     lhs = self.try_transform(lhs, name, lookups)
   1095                     final_lookup = lhs.get_lookup('exact')
-> 1096                 return final_lookup(lhs, rhs)
   1097             lhs = self.try_transform(lhs, name, lookups)
   1098             lookups = lookups[1:]

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/lookups.pyc in __init__(self, lhs, rhs)
     94     def __init__(self, lhs, rhs):
     95         self.lhs, self.rhs = lhs, rhs
---> 96         self.rhs = self.get_prep_lookup()
     97         if hasattr(self.lhs, 'get_bilateral_transforms'):
     98             bilateral_transforms = self.lhs.get_bilateral_transforms()

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/django/db/models/lookups.pyc in get_prep_lookup(self)
    132 
    133     def get_prep_lookup(self):
--> 134         return self.lhs.output_field.get_prep_lookup(self.lookup_name, self.rhs)
    135 
    136     def get_db_prep_lookup(self, value, connection):

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/netfields/fields.pyc in get_prep_lookup(self, lookup_type, value)
     39                 # Argument will be CIDR
     40                 return str(value)
---> 41             return self.get_prep_value(value)
     42 
     43         return super(_NetAddressField, self).get_prep_lookup(

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/netfields/fields.pyc in get_prep_value(self, value)
     48             return None
     49 
---> 50         return str(self.to_python(value))
     51 
     52     def get_db_prep_lookup(self, lookup_type, value, connection,

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/netfields/fields.pyc in to_python(self, value)
     24 
     25         try:
---> 26             return self.python_type()(value)
     27         except AddrFormatError as e:
     28             raise ValidationError(e)

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/netaddr/ip/__init__.pyc in __init__(self, addr, implicit_prefix, version, flags)
    921                 module = _ipv4
    922                 value, prefixlen = parse_ip_network(module, addr,
--> 923                     implicit_prefix, flags)
    924             except AddrFormatError:
    925                 try:

/home/esk/src/django-openipam/env/local/lib/python2.7/site-packages/netaddr/ip/__init__.pyc in parse_ip_network(module, addr, implicit_prefix, flags)
    816                 % module.family_name)
    817     else:
--> 818         raise TypeError('unexpected type %s for addr arg' % type(addr))
    819 
    820     if flags & NOHOST:

TypeError: unexpected type <type 'list'> for addr arg

comment:4 by Tim Graham, 9 years ago

Patch needs improvement: unset
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

How about the attached patched? If the field isn't relational, there's no need to do the cast in the first place. Not sure about a simple way to write a regression test.

by Tim Graham, 9 years ago

Attachment: 25150.diff added

comment:5 by Tim Graham, 9 years ago

Resolution: wontfix
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top