Opened 19 years ago
Closed 17 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: | dev |
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: | no | UI/UX: | no |
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)
Change History (17)
comment:1 by , 19 years ago
comment:3 by , 18 years ago
Cc: | added |
---|---|
Component: | Admin interface → 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:5 by , 18 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Matti, your idea sounds good. Want to provide a patch?
comment:6 by , 18 years ago
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 by , 18 years ago
Triage Stage: | Accepted → 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 by , 18 years ago
Triage Stage: | Ready for checkin → 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 by , 17 years ago
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.
by , 17 years ago
Attachment: | 708-1.diff added |
---|
comment:10 by , 17 years ago
Keywords: | qs-rf added |
---|---|
Version: | → SVN |
comment:11 by , 17 years ago
Keywords: | qs-rf removed |
---|
comment:12 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
follow-up: 14 comment:13 by , 17 years ago
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 by , 17 years ago
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 by , 17 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I've reverted [7151] (in [7160]) while we figure this out more completely; see the django-dev thread for more.
comment:16 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
The correct SQL would be this:
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).