Opened 7 years ago

Last modified 6 years ago

#28702 closed Bug

Field.db_type, Field.get_internal_type() and postgresql ::citext — at Version 2

Reported by: Дилян Палаузов Owned by:
Component: contrib.postgres Version: 1.11
Severity: Release blocker Keywords: citext postgres cast
Cc: Mads Jensen Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Дилян Палаузов)

According to https://docs.djangoproject.com/en/1.11/howto/custom-model-fields/#custom-database-types db_type() is used when constructing a WHERE clause that includes the model field – that is, when you retrieve data using QuerySet methods like get(), filter(), and exclude() and have the model field as an argument.

Then https://docs.djangoproject.com/en/1.11/howto/custom-model-fields/#emulating-built-in-field-types states, that get_internal_type() is used for correct usage of columns and during migrations.

This would imply that having a django.contrib.postgresq.fields.CICharField field, calling Model.objects.filter(field_contains="ABC") would be converted to 'WHERE field::citext LIKE "%ABC%"' but it is converted to 'WHERE field::text LIKE "%ABC%"', which does not work as expected, so one has to call filter(field_icontains="ABC") instead. The reason is that django/db/models/lookups.py:BuiltinLookup.process_lhs() passes the result of get_internal_type to db/backends/postgresql/operations.py:lookup_cast() and this is used to distinguish the right lookup.

My proposal for fixing the code for ::citext:

diff --git a/django/contrib/postgres/fields/citext.py b/django/contrib/postgres/fields/citext.py
--- a/django/contrib/postgres/fields/citext.py
+++ b/django/contrib/postgres/fields/citext.py
@@ -7,6 +7,9 @@ class CIText:
     def db_type(self, connection):
         return 'citext'
 
+    def get_internal_type(self):
+        return "CI" + super().get_internal_type()
+
 
 class CICharField(CIText, CharField):
     pass
diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py
--- a/django/db/backends/postgresql/operations.py
+++ b/django/db/backends/postgresql/operations.py
@@ -77,6 +77,8 @@ class DatabaseOperations(BaseDatabaseOperations):
                            'istartswith', 'endswith', 'iendswith', 'regex', 'iregex'):
             if internal_type in ('IPAddressField', 'GenericIPAddressField'):
                 lookup = "HOST(%s)"
+            elif internal_type in ('CICharField', 'CITextField'):
+                lookup = "%s::citext"
             else:
                 lookup = "%s::text"

diff --git a/django/db/backends/postgresql/schema.py b/django/db/backends/postgresql/schema.py
--- a/django/db/backends/postgresql/schema.py
+++ b/django/db/backends/postgresql/schema.py
@@ -112,6 +112,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
         if (old_field.db_index or old_field.unique) and (
             (old_type.startswith('varchar') and not new_type.startswith('varchar')) or
             (old_type.startswith('text') and not new_type.startswith('text'))
+            (old_type.startswith('citext') and not new_type.startswith('citext'))
         ):
             index_name = self._create_index_name(model._meta.db_table, [old_field.column], suffix='_like')
             self.execute(self._delete_constraint_sql(self.sql_delete_index, model, index_name))

Change History (2)

comment:1 by Tim Graham, 7 years ago

Cc: Mads Jensen added
Needs tests: set

Mads, since you did the initial implementation of CITTextField (#26610), do you want to evaluate the proposal? Backwards compatibility could be a concern at this point.

In the patch, the use of connection.features.interprets_empty_strings_as_nulls looks incorrect as this is only True for Oracle.

comment:2 by Дилян Палаузов, 7 years ago

Description: modified (diff)

I agree that overriding _alter_column_null_sql() makes no sense and have removed it from the description.

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