summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAnubhav Joshi <anubhav9042@gmail.com>2014-06-10 17:34:19 +0530
committerTim Graham <timograham@gmail.com>2014-06-17 16:16:02 -0400
commit91f1b6dcdc5da47d7794a55e3114820407a5bd62 (patch)
treef27e7176a448035362fdc95caa1e32013ef84ba1
parente4708385fde278fbbff5428f756632455c2bcaf5 (diff)
Fixed #13711 -- Model check added to ensure that auto-generated column name is within limits of the database.
Thanks russellm for report and Tim Graham for review.
-rw-r--r--django/db/backends/__init__.py3
-rw-r--r--django/db/backends/mysql/base.py1
-rw-r--r--django/db/models/base.py73
-rw-r--r--docs/ref/checks.txt5
-rw-r--r--docs/releases/1.8.txt22
-rw-r--r--tests/backends/models.py22
-rw-r--r--tests/backends/tests.py9
-rw-r--r--tests/invalid_models_tests/test_models.py143
8 files changed, 254 insertions, 24 deletions
diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py
index 869644bfac..8acfde014c 100644
--- a/django/db/backends/__init__.py
+++ b/django/db/backends/__init__.py
@@ -520,9 +520,6 @@ class BaseDatabaseFeatures(object):
# at the end of each save operation?
supports_forward_references = True
- # Does the backend allow very long model names without error?
- supports_long_model_names = True
-
# Is there a REAL datatype in addition to floats/doubles?
has_real_datatype = False
supports_subqueries_in_group_by = True
diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py
index 697a35a1dd..77369dcd8b 100644
--- a/django/db/backends/mysql/base.py
+++ b/django/db/backends/mysql/base.py
@@ -171,7 +171,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
has_select_for_update = True
has_select_for_update_nowait = False
supports_forward_references = False
- supports_long_model_names = False
# XXX MySQL DB-API drivers currently fail on binary data on Python 3.
supports_binary_field = six.PY2
supports_microsecond_precision = False
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 5142f93462..74bece1f5c 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -12,7 +12,7 @@ from django.conf import settings
from django.core import checks
from django.core.exceptions import (ObjectDoesNotExist,
MultipleObjectsReturned, FieldError, ValidationError, NON_FIELD_ERRORS)
-from django.db import (router, transaction, DatabaseError,
+from django.db import (router, connections, transaction, DatabaseError,
DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY)
from django.db.models.deletion import Collector
from django.db.models.fields import AutoField, FieldDoesNotExist
@@ -1068,6 +1068,7 @@ class Model(six.with_metaclass(ModelBase)):
if not cls._meta.swapped:
errors.extend(cls._check_fields(**kwargs))
errors.extend(cls._check_m2m_through_same_relationship())
+ errors.extend(cls._check_long_column_names())
clash_errors = cls._check_id_field() + cls._check_field_name_clashes()
errors.extend(clash_errors)
# If there are field name clashes, hide consequent column name
@@ -1451,6 +1452,76 @@ class Model(six.with_metaclass(ModelBase)):
)
return errors
+ @classmethod
+ def _check_long_column_names(cls):
+ """
+ Check that any auto-generated column names are shorter than the limits
+ for each database in which the model will be created.
+ """
+ errors = []
+ allowed_len = None
+ db_alias = None
+
+ # Find the minimum max allowed length among all specified db_aliases.
+ for db in settings.DATABASES.keys():
+ # skip databases where the model won't be created
+ if not router.allow_migrate(db, cls):
+ continue
+ connection = connections[db]
+ max_name_length = connection.ops.max_name_length()
+ if max_name_length is None:
+ continue
+ else:
+ if allowed_len is None:
+ allowed_len = max_name_length
+ db_alias = db
+ elif max_name_length < allowed_len:
+ allowed_len = max_name_length
+ db_alias = db
+
+ if allowed_len is None:
+ return errors
+
+ for f in cls._meta.local_fields:
+ _, column_name = f.get_attname_column()
+
+ # Check if auto-generated name for the field is too long
+ # for the database.
+ if (f.db_column is None and column_name is not None
+ and len(column_name) > allowed_len):
+ errors.append(
+ checks.Error(
+ 'Autogenerated column name too long for field "%s". '
+ 'Maximum length is "%s" for database "%s".'
+ % (column_name, allowed_len, db_alias),
+ hint="Set the column name manually using 'db_column'.",
+ obj=cls,
+ id='models.E018',
+ )
+ )
+
+ for f in cls._meta.local_many_to_many:
+ # Check if auto-generated name for the M2M field is too long
+ # for the database.
+ for m2m in f.rel.through._meta.local_fields:
+ _, rel_name = m2m.get_attname_column()
+ if (m2m.db_column is None and rel_name is not None
+ and len(rel_name) > allowed_len):
+ errors.append(
+ checks.Error(
+ 'Autogenerated column name too long for M2M field '
+ '"%s". Maximum length is "%s" for database "%s".'
+ % (rel_name, allowed_len, db_alias),
+ hint=("Use 'through' to create a separate model "
+ "for M2M and then set column_name using "
+ "'db_column'."),
+ obj=cls,
+ id='models.E019',
+ )
+ )
+
+ return errors
+
############################################
# HELPER FUNCTIONS (CURRIED MODEL METHODS) #
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index aedaa921db..85cd616253 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -58,6 +58,11 @@ Models
* **models.E016**: ``index_together/unique_together`` refers to field
``<field_name>`` which is not local to model ``<model>``.
* **models.E017**: Proxy model ``<model>`` contains model fields.
+* **models.E018**: Autogenerated column name too long for field ``<field>``.
+ Maximum length is ``<maximum length>`` for database ``<alias>``.
+* **models.E019**: Autogenerated column name too long for M2M field
+ ``<M2M field>``. Maximum length is ``<maximum length>`` for database
+ ``<alias>``.
Fields
~~~~~~
diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index c54e1d5ab0..7f3b4aa6c7 100644
--- a/docs/releases/1.8.txt
+++ b/docs/releases/1.8.txt
@@ -305,6 +305,28 @@ Now to implement the same behavior, you have to create an
``parser.add_argument`` to add any custom arguments, as parser is now an
:py:class:`argparse.ArgumentParser` instance.
+Model check ensures auto-generated column names are within limits specified by database
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+A field name that's longer than the column name length supported by a database
+can create problems. For example, with MySQL you'll get an exception trying to
+create the column, and with PostgreSQL the column name is truncated by the
+database (you may see a warning in the PostgreSQL logs).
+
+A model check has been introduced to better alert users to this scenario before
+the actual creation of database tables.
+
+If you have an existing model where this check seems to be a false positive,
+for example on PostgreSQL where the name was already being truncated, simply
+use :attr:`~django.db.models.Field.db_column` to specify the name that's being
+used.
+
+The check also applies to the columns generated in an implicit
+``ManyToManyField.through`` model. If you run into an issue there, use
+:attr:`~django.db.models.ManyToManyField.through` to create an explicit model
+and then specify :attr:`~django.db.models.Field.db_column` on its column(s)
+as needed.
+
Miscellaneous
~~~~~~~~~~~~~
diff --git a/tests/backends/models.py b/tests/backends/models.py
index 2a94c249ca..b550212d0b 100644
--- a/tests/backends/models.py
+++ b/tests/backends/models.py
@@ -4,7 +4,7 @@ from django.contrib.contenttypes.fields import (
GenericForeignKey, GenericRelation
)
from django.contrib.contenttypes.models import ContentType
-from django.db import models, connection
+from django.db import models
from django.utils.encoding import python_2_unicode_compatible
@@ -31,17 +31,15 @@ class SchoolClass(models.Model):
day = models.CharField(max_length=9, blank=True)
last_updated = models.DateTimeField()
-# Unfortunately, the following model breaks MySQL hard.
-# Until #13711 is fixed, this test can't be run under MySQL.
-if connection.features.supports_long_model_names:
- class VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ(models.Model):
- class Meta:
- # We need to use a short actual table name or
- # we hit issue #8548 which we're not testing!
- verbose_name = 'model_with_long_table_name'
- primary_key_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.AutoField(primary_key=True)
- charfield_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.CharField(max_length=100)
- m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True)
+
+class VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ(models.Model):
+ class Meta:
+ # We need to use a short actual table name or
+ # we hit issue #8548 which we're not testing!
+ verbose_name = 'model_with_long_table_name'
+ primary_key_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.AutoField(primary_key=True)
+ charfield_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.CharField(max_length=100)
+ m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True)
class Tag(models.Model):
diff --git a/tests/backends/tests.py b/tests/backends/tests.py
index 9742358a59..40b0b55082 100644
--- a/tests/backends/tests.py
+++ b/tests/backends/tests.py
@@ -379,26 +379,23 @@ class LongNameTest(TestCase):
check it is. Refs #8901.
"""
- @skipUnlessDBFeature('supports_long_model_names')
def test_sequence_name_length_limits_create(self):
"""Test creation of model with long name and long pk name doesn't error. Ref #8901"""
- models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.objects.create()
+ models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.objects.create()
- @skipUnlessDBFeature('supports_long_model_names')
def test_sequence_name_length_limits_m2m(self):
"""Test an m2m save of a model with a long name and a long m2m field name doesn't error as on Django >=1.2 this now uses object saves. Ref #8901"""
- obj = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.objects.create()
+ obj = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.objects.create()
rel_obj = models.Person.objects.create(first_name='Django', last_name='Reinhardt')
obj.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.add(rel_obj)
- @skipUnlessDBFeature('supports_long_model_names')
def test_sequence_name_length_limits_flush(self):
"""Test that sequence resetting as part of a flush with model with long name and long pk name doesn't error. Ref #8901"""
# A full flush is expensive to the full test, so we dig into the
# internals to generate the likely offending SQL and run it manually
# Some convenience aliases
- VLM = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ
+ VLM = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ
VLM_m2m = VLM.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.through
tables = [
VLM._meta.db_table,
diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py
index 9cbaa8449c..8550dc3065 100644
--- a/tests/invalid_models_tests/test_models.py
+++ b/tests/invalid_models_tests/test_models.py
@@ -1,13 +1,36 @@
# -*- encoding: utf-8 -*-
from __future__ import unicode_literals
+import unittest
+
+from django.conf import settings
from django.core.checks import Error
-from django.db import models
+from django.db import models, connections
from django.test.utils import override_settings
from .base import IsolatedModelsTestCase
+def get_max_column_name_length():
+ allowed_len = None
+ db_alias = None
+
+ for db in settings.DATABASES.keys():
+ connection = connections[db]
+ max_name_length = connection.ops.max_name_length()
+ if max_name_length is None:
+ continue
+ else:
+ if allowed_len is None:
+ allowed_len = max_name_length
+ db_alias = db
+ elif max_name_length < allowed_len:
+ allowed_len = max_name_length
+ db_alias = db
+
+ return (allowed_len, db_alias)
+
+
class IndexTogetherTests(IsolatedModelsTestCase):
def test_non_iterable(self):
@@ -252,6 +275,124 @@ class FieldNamesTests(IsolatedModelsTestCase):
]
self.assertEqual(errors, expected)
+ max_column_name_length, column_limit_db_alias = get_max_column_name_length()
+
+ @unittest.skipIf(max_column_name_length is None,
+ "The database doesn't have a column name length limit.")
+ def test_M2M_long_column_name(self):
+ """
+ #13711 -- Model check for long M2M column names when database has
+ column name length limits.
+ """
+ allowed_len, db_alias = get_max_column_name_length()
+
+ # A model with very long name which will be used to set relations to.
+ class VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz(models.Model):
+ title = models.CharField(max_length=11)
+
+ # Main model for which checks will be performed.
+ class ModelWithLongField(models.Model):
+ m2m_field = models.ManyToManyField(
+ VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz,
+ related_name="rn1"
+ )
+ m2m_field2 = models.ManyToManyField(
+ VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz,
+ related_name="rn2", through='m2msimple'
+ )
+ m2m_field3 = models.ManyToManyField(
+ VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz,
+ related_name="rn3",
+ through='m2mcomplex'
+ )
+ fk = models.ForeignKey(VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz, related_name="rn4")
+
+ # Models used for setting `through` in M2M field.
+ class m2msimple(models.Model):
+ id2 = models.ForeignKey(ModelWithLongField)
+
+ class m2mcomplex(models.Model):
+ id2 = models.ForeignKey(ModelWithLongField)
+
+ long_field_name = 'a' * (self.max_column_name_length + 1)
+ models.ForeignKey(
+ VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz
+ ).contribute_to_class(m2msimple, long_field_name)
+
+ models.ForeignKey(
+ VeryLongModelNamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz,
+ db_column=long_field_name
+ ).contribute_to_class(m2mcomplex, long_field_name)
+
+ errors = ModelWithLongField.check()
+
+ # First error because of M2M field set on the model with long name.
+ m2m_long_name = "verylongmodelnamezzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz_id"
+ expected = [
+ Error(
+ ('Autogenerated column name too long for M2M field "%s". '
+ 'Maximum length is "%s" for database "%s".'
+ % (m2m_long_name, self.max_column_name_length, self.column_limit_db_alias)),
+ hint=("Use 'through' to create a separate model for "
+ "M2M and then set column_name using 'db_column'."),
+ obj=ModelWithLongField,
+ id='models.E019',
+ )
+ ]
+
+ # Second error because the FK specified in the `through` model
+ # `m2msimple` has auto-genererated name longer than allowed.
+ # There will be no check errors in the other M2M because it
+ # specifies db_column for the FK in `through` model even if the actual
+ # name is longer than the limits of the database.
+ expected.append(
+ Error(
+ ('Autogenerated column name too long for M2M field "%s_id". '
+ 'Maximum length is "%s" for database "%s".'
+ % (long_field_name, self.max_column_name_length, self.column_limit_db_alias)),
+ hint=("Use 'through' to create a separate model for "
+ "M2M and then set column_name using 'db_column'."),
+ obj=ModelWithLongField,
+ id='models.E019',
+ )
+ )
+
+ self.assertEqual(errors, expected)
+
+ @unittest.skipIf(max_column_name_length is None,
+ "The database doesn't have a column name length limit.")
+ def test_local_field_long_column_name(self):
+ """
+ #13711 -- Model check for long column names
+ when database does not support long names.
+ """
+ allowed_len, db_alias = get_max_column_name_length()
+
+ class ModelWithLongField(models.Model):
+ title = models.CharField(max_length=11)
+
+ long_field_name = 'a' * (self.max_column_name_length + 1)
+ long_field_name2 = 'b' * (self.max_column_name_length + 1)
+ models.CharField(max_length=11).contribute_to_class(ModelWithLongField, long_field_name)
+ models.CharField(max_length=11, db_column='vlmn').contribute_to_class(ModelWithLongField, long_field_name2)
+
+ errors = ModelWithLongField.check()
+
+ # Error because of the field with long name added to the model
+ # without specifying db_column
+ expected = [
+ Error(
+ ('Autogenerated column name too long for field "%s". '
+ 'Maximum length is "%s" for database "%s".'
+ % (long_field_name, self.max_column_name_length, self.column_limit_db_alias)),
+ hint="Set the column name manually using 'db_column'.",
+ obj=ModelWithLongField,
+ id='models.E018',
+ )
+ ]
+
+ self.assertEqual(errors, expected)
+
def test_including_separator(self):
class Model(models.Model):
some__field = models.IntegerField()