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.
This commit is contained in:
Anubhav Joshi 2014-06-10 17:34:19 +05:30 committed by Tim Graham
parent e4708385fd
commit 91f1b6dcdc
8 changed files with 254 additions and 24 deletions

View File

@ -520,9 +520,6 @@ class BaseDatabaseFeatures(object):
# at the end of each save operation? # at the end of each save operation?
supports_forward_references = True 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? # Is there a REAL datatype in addition to floats/doubles?
has_real_datatype = False has_real_datatype = False
supports_subqueries_in_group_by = True supports_subqueries_in_group_by = True

View File

@ -171,7 +171,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
has_select_for_update = True has_select_for_update = True
has_select_for_update_nowait = False has_select_for_update_nowait = False
supports_forward_references = False supports_forward_references = False
supports_long_model_names = False
# XXX MySQL DB-API drivers currently fail on binary data on Python 3. # XXX MySQL DB-API drivers currently fail on binary data on Python 3.
supports_binary_field = six.PY2 supports_binary_field = six.PY2
supports_microsecond_precision = False supports_microsecond_precision = False

View File

@ -12,7 +12,7 @@ from django.conf import settings
from django.core import checks from django.core import checks
from django.core.exceptions import (ObjectDoesNotExist, from django.core.exceptions import (ObjectDoesNotExist,
MultipleObjectsReturned, FieldError, ValidationError, NON_FIELD_ERRORS) 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) DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY)
from django.db.models.deletion import Collector from django.db.models.deletion import Collector
from django.db.models.fields import AutoField, FieldDoesNotExist from django.db.models.fields import AutoField, FieldDoesNotExist
@ -1068,6 +1068,7 @@ class Model(six.with_metaclass(ModelBase)):
if not cls._meta.swapped: if not cls._meta.swapped:
errors.extend(cls._check_fields(**kwargs)) errors.extend(cls._check_fields(**kwargs))
errors.extend(cls._check_m2m_through_same_relationship()) 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() clash_errors = cls._check_id_field() + cls._check_field_name_clashes()
errors.extend(clash_errors) errors.extend(clash_errors)
# If there are field name clashes, hide consequent column name # If there are field name clashes, hide consequent column name
@ -1451,6 +1452,76 @@ class Model(six.with_metaclass(ModelBase)):
) )
return errors 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) # # HELPER FUNCTIONS (CURRIED MODEL METHODS) #

View File

@ -58,6 +58,11 @@ Models
* **models.E016**: ``index_together/unique_together`` refers to field * **models.E016**: ``index_together/unique_together`` refers to field
``<field_name>`` which is not local to model ``<model>``. ``<field_name>`` which is not local to model ``<model>``.
* **models.E017**: Proxy model ``<model>`` contains model fields. * **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 Fields
~~~~~~ ~~~~~~

View File

@ -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 ``parser.add_argument`` to add any custom arguments, as parser is now an
:py:class:`argparse.ArgumentParser` instance. :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 Miscellaneous
~~~~~~~~~~~~~ ~~~~~~~~~~~~~

View File

@ -4,7 +4,7 @@ from django.contrib.contenttypes.fields import (
GenericForeignKey, GenericRelation GenericForeignKey, GenericRelation
) )
from django.contrib.contenttypes.models import ContentType 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 from django.utils.encoding import python_2_unicode_compatible
@ -31,10 +31,8 @@ class SchoolClass(models.Model):
day = models.CharField(max_length=9, blank=True) day = models.CharField(max_length=9, blank=True)
last_updated = models.DateTimeField() last_updated = models.DateTimeField()
# Unfortunately, the following model breaks MySQL hard.
# Until #13711 is fixed, this test can't be run under MySQL. class VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ(models.Model):
if connection.features.supports_long_model_names:
class VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ(models.Model):
class Meta: class Meta:
# We need to use a short actual table name or # We need to use a short actual table name or
# we hit issue #8548 which we're not testing! # we hit issue #8548 which we're not testing!

View File

@ -379,26 +379,23 @@ class LongNameTest(TestCase):
check it is. Refs #8901. check it is. Refs #8901.
""" """
@skipUnlessDBFeature('supports_long_model_names')
def test_sequence_name_length_limits_create(self): def test_sequence_name_length_limits_create(self):
"""Test creation of model with long name and long pk name doesn't error. Ref #8901""" """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): 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""" """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') rel_obj = models.Person.objects.create(first_name='Django', last_name='Reinhardt')
obj.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.add(rel_obj) obj.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.add(rel_obj)
@skipUnlessDBFeature('supports_long_model_names')
def test_sequence_name_length_limits_flush(self): 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""" """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 # 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 # internals to generate the likely offending SQL and run it manually
# Some convenience aliases # Some convenience aliases
VLM = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ VLM = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ
VLM_m2m = VLM.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.through VLM_m2m = VLM.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.through
tables = [ tables = [
VLM._meta.db_table, VLM._meta.db_table,

View File

@ -1,13 +1,36 @@
# -*- encoding: utf-8 -*- # -*- encoding: utf-8 -*-
from __future__ import unicode_literals from __future__ import unicode_literals
import unittest
from django.conf import settings
from django.core.checks import Error 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 django.test.utils import override_settings
from .base import IsolatedModelsTestCase 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): class IndexTogetherTests(IsolatedModelsTestCase):
def test_non_iterable(self): def test_non_iterable(self):
@ -252,6 +275,124 @@ class FieldNamesTests(IsolatedModelsTestCase):
] ]
self.assertEqual(errors, expected) 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): def test_including_separator(self):
class Model(models.Model): class Model(models.Model):
some__field = models.IntegerField() some__field = models.IntegerField()