[Django] #24817: Renaming a model field that has null=False makes it nullable in MySQL
Django
----------------------------+--------------------
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Steps to reproduce:
1. Create a simple model:
{{{
class TestModel(models.Model):
value_a = models.IntegerField(null=False)
}}}
2. '''./manage.py makemigrations'''
3. '''./manage.py migrate'''
4. So far so good:
{{{
mysql> describe testy_testmodel;
+---------+---------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+---------+------+-----+---------+----------------+
| id | int(11) | NO | PRI | NULL | auto_increment |
| value_a | int(11) | NO | | NULL | |
+---------+---------+------+-----+---------+----------------+
2 rows in set (0.00 sec)
}}}
5. Change model field name '''value_a''' -> '''value_b''':
{{{
class TestModel(models.model):
value_b = models.IntegerField(null=False)
}}}
6. '''./manage.py makemigrations'''
7. '''./manage.py migrate'''
8. '''NOT NULL''' for '''value_b''' has bee lost:
{{{
mysql> describe testy_testmodel;
+---------+---------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+---------+------+-----+---------+----------------+
| id | int(11) | NO | PRI | NULL | auto_increment |
| value_b | int(11) | YES | | NULL | |
+---------+---------+------+-----+---------+----------------+
2 rows in set (0.00 sec)
}}}
The '''ALTER-'''statement django produces is
{{{
ALTER TABLE `testy_testmodel` CHANGE `value_a` `value_b` integer;
}}}
...and it should contain the '''NOT NULL''', but it doesn't.
This affects other '''ALTER TABLE'''-statements too, e.g. if changing
field type from''' IntegerField''' to '''BigIntergerField'''. The bug is
present in 1.7 as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/24817>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by murfi):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
New description:
Steps to reproduce:
1. Create a simple model:
{{{
class TestModel(models.Model):
value_a = models.IntegerField(null=False)
}}}
2. '''./manage.py makemigrations'''
3. '''./manage.py migrate'''
4. So far so good:
{{{
mysql> describe testy_testmodel;
+---------+---------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+---------+------+-----+---------+----------------+
| id | int(11) | NO | PRI | NULL | auto_increment |
| value_a | int(11) | NO | | NULL | |
+---------+---------+------+-----+---------+----------------+
2 rows in set (0.00 sec)
}}}
5. Change model field name '''value_a''' -> '''value_b''':
{{{
class TestModel(models.model):
value_b = models.IntegerField(null=False)
}}}
6. '''./manage.py makemigrations'''
7. '''./manage.py migrate'''
8. '''NOT NULL''' for '''value_b''' has been lost:
{{{
mysql> describe testy_testmodel;
+---------+---------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+---------+------+-----+---------+----------------+
| id | int(11) | NO | PRI | NULL | auto_increment |
| value_b | int(11) | YES | | NULL | |
+---------+---------+------+-----+---------+----------------+
2 rows in set (0.00 sec)
}}}
The '''ALTER-'''statement django produces is
{{{
ALTER TABLE `testy_testmodel` CHANGE `value_a` `value_b` integer;
}}}
...and it should contain the '''NOT NULL''', but it doesn't.
This affects other '''ALTER TABLE'''-statements too, e.g. if changing
field type from''' IntegerField''' to '''BigIntergerField'''. The bug is
present in 1.7 as well.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:1>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by alasdairnicol):
* status: new => closed
* resolution: => duplicate
Comment:
This looks like a duplicate of #24595, which is fixed in 1.7.8 and 1.8.1.
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:2>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by alasdairnicol):
* cc: alasdair@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:3>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by murfi):
* status: closed => new
* resolution: duplicate =>
Old description:
> Steps to reproduce:
>
> 1. Create a simple model:
> {{{
> class TestModel(models.Model):
> value_a = models.IntegerField(null=False)
> }}}
> 2. '''./manage.py makemigrations'''
> 3. '''./manage.py migrate'''
> 4. So far so good:
> {{{
> mysql> describe testy_testmodel;
> +---------+---------+------+-----+---------+----------------+
> | Field | Type | Null | Key | Default | Extra |
> +---------+---------+------+-----+---------+----------------+
> | id | int(11) | NO | PRI | NULL | auto_increment |
> | value_a | int(11) | NO | | NULL | |
> +---------+---------+------+-----+---------+----------------+
> 2 rows in set (0.00 sec)
> }}}
> 5. Change model field name '''value_a''' -> '''value_b''':
> {{{
> class TestModel(models.model):
> value_b = models.IntegerField(null=False)
> }}}
> 6. '''./manage.py makemigrations'''
> 7. '''./manage.py migrate'''
> 8. '''NOT NULL''' for '''value_b''' has been lost:
> {{{
> mysql> describe testy_testmodel;
> +---------+---------+------+-----+---------+----------------+
> | Field | Type | Null | Key | Default | Extra |
> +---------+---------+------+-----+---------+----------------+
> | id | int(11) | NO | PRI | NULL | auto_increment |
> | value_b | int(11) | YES | | NULL | |
> +---------+---------+------+-----+---------+----------------+
> 2 rows in set (0.00 sec)
> }}}
>
> The '''ALTER-'''statement django produces is
> {{{
> ALTER TABLE `testy_testmodel` CHANGE `value_a` `value_b` integer;
> }}}
> ...and it should contain the '''NOT NULL''', but it doesn't.
>
> This affects other '''ALTER TABLE'''-statements too, e.g. if changing
> field type from''' IntegerField''' to '''BigIntergerField'''. The bug is
> present in 1.7 as well.
New description:
Steps to reproduce:
1. Create a simple model:
{{{
class TestModel(models.Model):
value_a = models.IntegerField(null=False)
}}}
2. '''./manage.py makemigrations'''
3. '''./manage.py migrate'''
4. So far so good:
{{{
mysql> describe testy_testmodel;
+---------+---------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+---------+------+-----+---------+----------------+
| id | int(11) | NO | PRI | NULL | auto_increment |
| value_a | int(11) | NO | | NULL | |
+---------+---------+------+-----+---------+----------------+
2 rows in set (0.00 sec)
}}}
5. Change model field name '''value_a''' -> '''value_b''':
{{{
class TestModel(models.model):
value_b = models.IntegerField(null=False)
}}}
6. '''./manage.py makemigrations'''
7. '''./manage.py migrate'''
8. '''NOT NULL''' for '''value_b''' has been lost:
{{{
mysql> describe testy_testmodel;
+---------+---------+------+-----+---------+----------------+
| Field | Type | Null | Key | Default | Extra |
+---------+---------+------+-----+---------+----------------+
| id | int(11) | NO | PRI | NULL | auto_increment |
| value_b | int(11) | YES | | NULL | |
+---------+---------+------+-----+---------+----------------+
2 rows in set (0.00 sec)
}}}
The '''ALTER-'''statement django produces is
{{{
ALTER TABLE `testy_testmodel` CHANGE `value_a` `value_b` integer;
}}}
...and it should contain the '''NOT NULL''', but it doesn't.
'''Edit''': Similar bug when changing field type was fixed in #24595. The
bug is still present in 1.8.1 when renaming a field.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:4>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by murfi):
True the comment in the end related to changing field type was invalid and
fixed in #24595. The example case is still reproducible with
Django==1.8.1.
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:5>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by alasdairnicol):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Apologies for incorrectly closing as a duplicate before. I can reproduce
in Django 1.7.8, 1.8.1 and master.
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:6>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
We've missed the rename use case while fixing #24595 :-(
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:7>
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* Attachment "ticket_24817.patch" added.
Test to reproduce the issue
Django
Reporter: murfi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* cc: me@… (added)
Comment:
I just attached a patch with tests which reproduces the issue.
I'm also working on fixing issue
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:8>
Django
Reporter: murfi | Owner: coldmind
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* owner: nobody => coldmind
* status: new => assigned
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/4714
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:9>
Django
Reporter: murfi | Owner: coldmind
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by coldmind):
PR against master - https://github.com/django/django/pull/4715
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:10>
Django
Reporter: murfi | Owner: coldmind
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
Claude, look good to you?
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:11>
Django
Reporter: murfi | Owner: coldmind
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Yes, sure! Thanks Andriy.
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:12>
Django
Reporter: murfi | Owner: coldmind
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"80ad5472ce4b6ba6e94227422d0727371e97cdf0" 80ad547]:
{{{
#!CommitTicketReference repository=""
revision="80ad5472ce4b6ba6e94227422d0727371e97cdf0"
Fixed #24817 -- Prevented loss of null info in MySQL field renaming.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:13>
Django
Reporter: murfi | Owner: coldmind
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"f65d4db8a805e5184898563542fcb5d459273582" f65d4db]:
{{{
#!CommitTicketReference repository=""
revision="f65d4db8a805e5184898563542fcb5d459273582"
[1.8.x] Fixed #24817 -- Prevented loss of null info in MySQL field
renaming.
Backport of 80ad5472ce4b6ba6e94227422d0727371e97cdf0 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:14>
Django
Reporter: murfi | Owner: coldmind
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"927d90ee1ee4aa630a5a879b5fd75aa03a3341f7" 927d90ee]:
{{{
#!CommitTicketReference repository=""
revision="927d90ee1ee4aa630a5a879b5fd75aa03a3341f7"
[1.7.x] Fixed #24817 -- Prevented loss of null info in MySQL field
renaming.
Backport of 80ad5472ce4b6ba6e94227422d0727371e97cdf0 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24817#comment:15>