Querying CIText fields should use ::citext type cast

[Дилян Палаузов]

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))

[Tim Graham]

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.

[Дилян Палаузов]

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

[Mads Jensen]

I think the change is necessary to make. The fact that citext doesn't behave like described in PostgreSQL's documentation is a flaw in the original implementation. I'm not sure about the "Backwards compatibility" concerns, but haven't other features been bug-fixed in minor releases as well, which would qualify for this as well?

I checked the query for array_field__contains=['joe'] which is casted correctly.

[Mads Jensen]

I think this issue highlights that the section Custom database types in docs/howto/custom-model-fields.txt needs to be adjusted, as db_type is referenced for filter and exclude but is not always used for casting.

The test test_lookup_cast for PostgreSQL will also need to be adjusted.

[Дилян Палаузов]

If by adjusting docs/howto/custom-model-fields.txt and test_lookup_cast is meant something different than this, please modify the files according to your opinion.

diff --git a/docs/howto/custom-model-fields.txt b/docs/howto/custom-model-fields.txt
--- a/docs/howto/custom-model-fields.txt
+++ b/docs/howto/custom-model-fields.txt
@@ -405,7 +405,7 @@ For example::
 The :meth:`~Field.db_type` and :meth:`~Field.rel_db_type` methods are called by
 Django when the framework constructs the ``CREATE TABLE`` statements for your
 application -- that is, when you first create your tables. The methods are also
-called when constructing a ``WHERE`` clause that includes the model field --
+called, together with :meth:`.get_internal_type`, 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. They
 are not called at any other time, so it can afford to execute slightly complex
diff --git a/tests/backends/postgresql/tests.py b/tests/backends/postgresql/tests.py
--- a/tests/backends/postgresql/tests.py
+++ b/tests/backends/postgresql/tests.py
@@ -138,6 +138,8 @@ class Tests(TestCase):
         for lookup in lookups:
             with self.subTest(lookup=lookup):
                 self.assertIn('::text', do.lookup_cast(lookup))
+                self.assertIn('::citext', do.lookup_cast(lookup, 'CICharField'))
+                self.assertIn('::citext', do.lookup_cast(lookup, 'CITextField'))
 
     def test_correct_extraction_psycopg2_version(self):
         from django.db.backends.postgresql.base import psycopg2_version

[Mads Jensen]

​PR

I couldn't do a git commit --author as I don't know the email of Дилян Палаузов. As highlighted in the PR, it mixes contrib.postgres into the normal PostgreSQL backend.

[Дилян Палаузов]

You can use as email address dpa-django with domain aegee.org, but I do not insist on this.

[Tim Graham <timograham@…>]

In f0a68c2:

Fixed #28702 -- Made query lookups for CIText fields use citext.

[Tim Graham <timograham@…>]

In 4d3b8e19:

[2.0.x] Fixed #28702 -- Made query lookups for CIText fields use citext.

Backport of f0a68c25118786d47041d0a435b2afa953be3c86 from master

[Tim Graham <timograham@…>]

In 3545e844:

[1.11.x] Fixed #28702 -- Made query lookups for CIText fields use citext.

Backport of f0a68c25118786d47041d0a435b2afa953be3c86 from master