#28702: Field.db_type, Field.get_internal_type() and postgresql ::citext
----------------------------------+--------------------------------------
Reporter: Дилян Палаузов | Owner: (none)
Type: Uncategorized | Status: new
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------
Old description:
> 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))
> @@ -131,3 +132,11 @@ class
> DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
> if old_field.unique and not (new_field.db_index or
> new_field.unique):
> index_to_remove =
> 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_to_remove))
> +
> +
> + def _alter_column_null_sql(self, model, old_field, new_field):
> + if (self.connection.features.interprets_empty_strings_as_nulls
> and
> + new_field.get_internal_type() in ("CICharField",
> "CITextField")):
> + # The field is nullable in the database anyway, leave it
> alone.
> + return
> + return super()._alter_column_null_sql(model, old_field,
> new_field
> }}}
New description:
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))
}}}
--
Comment (by Дилян Палаузов):
I agree that overriding _alter_column_null_sql() makes no sense and have
removed it from the description.
--
Ticket URL: <https://code.djangoproject.com/ticket/28702#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/072.15d144da2a68d6ae8107b0e200ee277f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
