Opened 9 years ago

Closed 7 years ago

#708 closed defect (fixed)

search for meta.IPAddressField with postgresql backend is broken (admin)

Reported by: jhernandez Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: normal Keywords: postgres
Cc: kilian.cavalotti@…, mattimustang@…, smileychris+django@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In the admin interface the search for meta.IPAddressField with a postgresql backend is broken due to a missing cast to text from inet fields:

My model is like this:

class ConnectionType(meta.Model):
    name = meta.CharField(maxlength=20)

    def __repr__(self):
        return self.name

class Connection(meta.Model):
    connection_type = meta.ForeignKey(ConnectionType)
    source_ip = meta.IPAddressField()
    source_port = meta.PositiveIntegerField()
    destination_ip = meta.IPAddressField()
    destination_port = meta.PositiveIntegerField()
    state = meta.CharField(maxlength=20)

    def __repr__(self):
        return "%s:%d - %s:%d (%s)" % (self.source_ip, self.source_port, self.destination_ip, self.destination_port,
                                       self.state)

    class META:
        admin = meta.Admin(
            list_display = ('connection_type', 'source_ip', 'source_port', 'destination_ip', 'destination_port', 'state'),
            list_filter = ['connection_type'],
            search_fields = ['source_ip', 'destination_ip'],
            )

in the admin inteface the search for an IPv4 address fails with the following traceback:

There's been an error:

Traceback (most recent call last):

  File "/usr/lib/python2.3/site-packages/django/core/handlers/base.py", line 71, in get_response
    response = callback(request, **param_dict)

  File "/usr/lib/python2.3/site-packages/django/contrib/admin/views/decorators.py", line 49, in _checklogin
    return view_func(request, *args, **kwargs)

  File "/usr/lib/python2.3/site-packages/django/contrib/admin/views/main.py", line 180, in change_list
    result_count = p.hits

  File "/usr/lib/python2.3/site-packages/django/core/paginator.py", line 67, in _get_hits
    self._hits = getattr(self.module, self.count_method)(**order_args)

  File "/usr/lib/python2.3/site-packages/django/utils/functional.py", line 3, in _curried
    return args[0](*(args[1:]+moreargs), **dict(kwargs.items() + morekwargs.items()))

  File "/usr/lib/python2.3/site-packages/django/core/meta/__init__.py", line 1144, in function_get_count
    cursor.execute("SELECT COUNT(*)" + sql, params)

  File "/usr/lib/python2.3/site-packages/django/core/db/base.py", line 10, in execute
    result = self.cursor.execute(sql, params)

ProgrammingError: ERROR:  operator does not exist: inet ~~* "unknown"
HINT:  No operator matches the given name and argument type(s). You may need to add explicit type casts.

SELECT COUNT(*) FROM firewall_connections WHERE (firewall_connections.source_ip ILIKE '%10.10.1%' OR firewall_connections.destination_ip ILIKE '%10.10.1%')

In the following thread of the post to the pgsql-bugs mailing list is the complete explanation of this "bug":

http://archives.postgresql.org/pgsql-bugs/2003-11/msg00165.php

on this message shows the proper query that should be used:

http://archives.postgresql.org/pgsql-bugs/2003-11/msg00167.php

this is the main reason:

http://archives.postgresql.org/pgsql-bugs/2003-11/msg00170.php

Attachments (1)

708-1.diff (660 bytes) - added by mattmcc 7 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 9 years ago by adrian

The correct SQL would be this:

select * from <table> where CAST(<field> as text) ilike <pattern>

In order to fix this, we would need the DB wrapper to have an extra per-field hook that would process values before using them in a LIKE statement. And on top of that, this would have to be database-engine-specific (only Postgres).

comment:2 Changed 9 years ago by adrian

#2149 was a duplicate.

comment:3 Changed 9 years ago by mattimustang@…

  • Cc kilian.cavalotti@… mattimustang@… added
  • Component changed from Admin interface to Database wrapper

A simple fix for this would be to make the IPAddressField a char(15) like every other backend instead of being a special case as it is now of being {inet.
In my own app I ended up using a CharField instead of IPAddressField because of this bug.

comment:4 Changed 8 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

comment:5 Changed 8 years ago by SmileyChris

  • Cc smileychris+django@… added
  • Triage Stage changed from Unreviewed to Accepted

Matti, your idea sounds good. Want to provide a patch?

comment:6 Changed 8 years ago by Matthew Flanagan <mattimustang@…>

  • Has patch set
  • Keywords postgres added

Chris,

This will be a backwards incompatible change for those users who are using postgres and IPAddressFields.

Index: django/db/backends/postgresql/creation.py
===================================================================
--- django/db/backends/postgresql/creation.py   (revision 4347)
+++ django/db/backends/postgresql/creation.py   (working copy)
@@ -14,7 +14,7 @@
     'FloatField':        'numeric(%(max_digits)s, %(decimal_places)s)',
     'ImageField':        'varchar(100)',
     'IntegerField':      'integer',
-    'IPAddressField':    'inet',
+    'IPAddressField':    'char(15)',
     'ManyToManyField':   None,
     'NullBooleanField':  'boolean',
     'OneToOneField':     'integer',

comment:7 Changed 8 years ago by SmileyChris

  • Triage Stage changed from Accepted to Ready for checkin

Thanks Matthew.

Core: I'm changing to "ready" for now. If you want more discussion, switch back to "design decision needed"

comment:8 Changed 8 years ago by jacob

  • Triage Stage changed from Ready for checkin to Design decision needed

I'm setting this back to design decision needed since I'm not sure how to fix this exactly. We could:

# Disallow IP fields from searching
# Switch to char(15)
# Add casting

I'm not sure which is the right choice.

comment:9 Changed 7 years ago by mattmcc

Unless I'm missing something, implementing the field and backend specific cast has become a lot easier than when the ticket was opened. This would surely be better than dropping support for the inet field type.

Changed 7 years ago by mattmcc

comment:10 Changed 7 years ago by jacob

  • Keywords qs-rf added
  • Version set to SVN

comment:11 Changed 7 years ago by mtredinnick

  • Keywords qs-rf removed

comment:12 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [7151]) Fixed #708 -- Fixed searching within IP fields on PostgreSQL.
Based on a patch from Matt McClanahan.

comment:13 follow-up: Changed 7 years ago by mtredinnick

The commit in [7151] isn't the perfect solution, since it does the cast always, but it retains backwards-compatibility and fixes the problem in a reasonable fashion (we don't support field-vs-field comparisons yet). It'll do for now.

comment:14 in reply to: ↑ 13 Changed 7 years ago by adamcik

As I mentioned on the django-developer list I would recommend using "host(ip)" instead of "CAST(ip AS text)" as the cast will return the ip as an inet address, eg. 10.0.0.1/32, while the host function will return the ip part.

comment:15 Changed 7 years ago by jacob

  • Resolution fixed deleted
  • Status changed from closed to reopened

I've reverted [7151] (in [7160]) while we figure this out more completely; see the django-dev thread for more.

comment:16 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [7161]) Re-enable substring lookups for IP address fields in Postgres using HOST() Thanks for the suggestion, Thomas Adamcik. Fixes #708.

Note: See TracTickets for help on using tickets.
Back to Top