[Django] #28120: CharField: if max_length=True (or False): makemigrations generates a migration and migrate fails
Django
migration and migrate fails
-------------------------------------+-------------------------------------
Reporter: Carles | Owner: nobody
Pina Estany |
Type: Bug | Status: new
Component: Database | Version: 1.11
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
If a model has a field like:
{{{
surname = models.CharField(max_length=True, null=True, blank=True)
}}}
python manage.py makemigrations writes a migration file but python
manage.py migrate fails with an error:
{{{
carles@pinux:~/git/django-test/mysite$ python3 manage.py migrate
Operations to perform:
Apply all migrations: admin, app01, auth, contenttypes, sessions
Running migrations:
Applying app01.0002_auto_20170424_1440... OK
Applying app01.0003_auto_20170424_1446...Traceback (most recent call
last):
File "/home/carles/git/django/django/db/backends/utils.py", line 60, in
execute
return self.cursor.execute(sql)
File "/home/carles/git/django/django/db/backends/sqlite3/base.py", line
289, in execute
return Database.Cursor.execute(self, query)
sqlite3.OperationalError: near "True": syntax error
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "manage.py", line 15, in <module>
execute_from_command_line(sys.argv)
File "/home/carles/git/django/django/core/management/__init__.py", line
354, in execute_from_command_line
utility.execute()
File "/home/carles/git/django/django/core/management/__init__.py", line
348, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/carles/git/django/django/core/management/base.py", line 280,
in run_from_argv
self.execute(*args, **cmd_options)
File "/home/carles/git/django/django/core/management/base.py", line 327,
in execute
output = self.handle(*args, **options)
File
"/home/carles/git/django/django/core/management/commands/migrate.py", line
200, in handle
fake_initial=fake_initial,
File "/home/carles/git/django/django/db/migrations/executor.py", line
113, in migrate
state = self._migrate_all_forwards(state, plan, full_plan, fake=fake,
fake_initial=fake_initial)
File "/home/carles/git/django/django/db/migrations/executor.py", line
143, in _migrate_all_forwards
state = self.apply_migration(state, migration, fake=fake,
fake_initial=fake_initial)
File "/home/carles/git/django/django/db/migrations/executor.py", line
240, in apply_migration
state = migration.apply(state, schema_editor)
File "/home/carles/git/django/django/db/migrations/migration.py", line
122, in apply
operation.database_forwards(self.app_label, schema_editor, old_state,
project_state)
File
"/home/carles/git/django/django/db/migrations/operations/fields.py", line
210, in database_forwards
schema_editor.alter_field(from_model, from_field, to_field)
File "/home/carles/git/django/django/db/backends/base/schema.py", line
490, in alter_field
old_db_params, new_db_params, strict)
File "/home/carles/git/django/django/db/backends/sqlite3/schema.py",
line 258, in _alter_field
self._remake_table(model, alter_field=(old_field, new_field))
File "/home/carles/git/django/django/db/backends/sqlite3/schema.py",
line 195, in _remake_table
self.create_model(temp_model)
File "/home/carles/git/django/django/db/backends/base/schema.py", line
290, in create_model
self.execute(sql, params or None)
File "/home/carles/git/django/django/db/backends/base/schema.py", line
109, in execute
cursor.execute(sql, params)
File "/home/carles/git/django/django/db/backends/utils.py", line 77, in
execute
return super().execute(sql, params)
File "/home/carles/git/django/django/db/backends/utils.py", line 62, in
execute
return self.cursor.execute(sql, params)
File "/home/carles/git/django/django/db/utils.py", line 90, in __exit__
raise dj_exc_value.with_traceback(traceback)
File "/home/carles/git/django/django/db/backends/utils.py", line 60, in
execute
return self.cursor.execute(sql)
File "/home/carles/git/django/django/db/backends/sqlite3/base.py", line
289, in execute
return Database.Cursor.execute(self, query)
django.db.utils.OperationalError: near "True": syntax error
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28120>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Carles Pina Estany
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:1>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 1.11 => master
* stage: Unreviewed => Ready for checkin
Comment:
[https://github.com/django/django/pull/8400 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:2>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"9f2e8b5bb79722ccafa3c4d6816bc847be1f59f9" 9f2e8b5]:
{{{
#!CommitTicketReference repository=""
revision="9f2e8b5bb79722ccafa3c4d6816bc847be1f59f9"
Fixed #28120 -- Checked that CharField.max_length is not a boolean.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:3>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Дилян Палаузов (added)
Comment:
What do max_length = True and max_length=False mean?
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:4>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
They are invalid values.
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:5>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Дилян Палаузов this was required because `issubclass(bool, int)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:6>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Дилян Палаузов):
What about ensuring that max_length is an int, and not bool this way:
{{{
diff --git a/django/db/models/fields/__init__.py
b/django/db/models/fields/__init__.py
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -1045,8 +1045,7 @@ class CharField(Field):
id='fields.E120',
)
]
- elif (not isinstance(self.max_length, int) or
isinstance(self.max_length, bool) or
- self.max_length <= 0):
+ elif type(self.max_length) != int or self.max_length <= 0:
return [
checks.Error(
"'max_length' must be a positive integer.",
}}}
There are more places in the code that check for {{{not isinstance(...,
int)}}}.
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:7>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
While uncommon that would prevent user defined `int` subclasses to work
appropriately.
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:8>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Дилян Палаузов):
Why do you want to allow subclasses of {{{int}}} to be passed as
max_length, but {{{bool}}}, which is a subclass of {{{int}}}, shall be
excluded?
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:9>
Django
migration and migrate fails
-------------------------------------+-------------------------------------
| Pina Estany
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
This is all getting into bike shedding territory but I assume that if a
user is advanced enough to subclass `int` and pass it to `max_length` they
know what they are doing.
On the opposite passing a `max_length=bool` is highly likely to be a
beginner mistake. The fact `bool` subclasses `int` is really just a relic
of the past which I assume most Python users are not aware of. In my case
at least I wasn't aware of it before tackling this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/28120#comment:10>