Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24595 closed Bug (fixed)

alter field type on MySQL "forgets" nullability (and more)

Reported by: Simon Percivall Owned by: Shai Berger <shai@…>
Component: Migrations Version: 1.7
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A migration changing field type on MySQL will remove "NOT NULL" (and every other attribute) from the field. This is because MODIFY COLUMN on MySQL requires the whole field definition to be repeated, not just the type.

So for instance:

class T(models.Model):
    field = models.SmallIntegerField()

will create a table:

CREATE TABLE `app_t` (
  ...
  `field` smallint(6) NOT NULL,
  ...
) ENGINE=InnoDB

Changing the field definition to:

...
    field = models.IntegerField()
...

will create a migration:

...
migrations.AlterField(
    model_name='t',
    name='field',
    field=models.IntegerField(),
)
...

resulting in the SQL

ALTER TABLE `app_t` MODIFY `field` integer

resulting in the table:

CREATE TABLE `app_t` (
  ...
  ``field` int(11) DEFAULT NULL,
  ...
)

This applies to 1.7 through 1.8.

Attachments (1)

24595-test.diff (1.1 KB) - added by Claude Paroz 4 years ago.
test failure

Download all attachments as: .zip

Change History (19)

comment:1 Changed 4 years ago by Tim Graham

Could you check if this is a duplicate of #23678?

Changed 4 years ago by Claude Paroz

Attachment: 24595-test.diff added

test failure

comment:2 Changed 4 years ago by Claude Paroz

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:3 Changed 4 years ago by Claude Paroz

Owner: changed from nobody to Claude Paroz
Status: newassigned

comment:4 Changed 4 years ago by Claude Paroz

Has patch: set
Owner: Claude Paroz deleted
Status: assignednew

comment:5 Changed 4 years ago by Simon Charette

Triage Stage: AcceptedReady for checkin

comment:6 Changed 4 years ago by Claude Paroz

Are schema corruption bugs considered having the same severity as data loss bugs? I'd say yes, and if it is confirmed, the patch should then also be backported to 1.7.x. Thoughts?

Last edited 4 years ago by Claude Paroz (previous) (diff)

comment:7 Changed 4 years ago by Tim Graham

Okay with me.

comment:8 Changed 4 years ago by Claude Paroz <claude@…>

Owner: set to Claude Paroz <claude@…>
Resolution: fixed
Status: newclosed

In 02260ea:

Fixed #24595 -- Prevented loss of null info in MySQL field alteration

Thanks Simon Percivall for the report, and Simon Charette and Tim
Graham for the reviews.

comment:9 Changed 4 years ago by Claude Paroz <claude@…>

In bbfcd961:

[1.8.x] Fixed #24595 -- Prevented loss of null info in MySQL field alteration

Thanks Simon Percivall for the report, and Simon Charette and Tim
Graham for the reviews.
Backport of 02260ea3f6 from master.

comment:10 Changed 4 years ago by Claude Paroz <claude@…>

In ada0845d:

[1.7.x] Fixed #24595 -- Prevented loss of null info in MySQL field alteration

Thanks Simon Percivall for the report, and Simon Charette and Tim
Graham for the reviews.
Backport of 02260ea3f61b from master.

comment:11 Changed 4 years ago by Claude Paroz

Resolution: fixed
Status: closednew

The failures on Oracle shown in Jenkins tend to show that Oracle may also be affected by this bug. I'll hand this over to a more knowledgeable person, though.

comment:12 Changed 4 years ago by Claude Paroz

Owner: changed from Claude Paroz <claude@…> to Claude Paroz
Status: newassigned

comment:13 Changed 4 years ago by Claude Paroz

Owner: Claude Paroz deleted
Status: assignednew

comment:14 Changed 4 years ago by Claude Paroz

Has patch: unset
Triage Stage: Ready for checkinAccepted

comment:15 Changed 4 years ago by Shai Berger <shai@…>

Owner: set to Shai Berger <shai@…>
Resolution: fixed
Status: newclosed

In 773ec512:

[1.7.x] Fixed #24595 Oracle test failure

The only problem for Oracle was the test, which tested nullity on
text/char fields -- but Oracle interprets_empty_strings_as_null.

Backport of d5a0acc from master

comment:16 Changed 4 years ago by Shai Berger <shai@…>

In 8363f217:

[1.8.x] Fixed #24595 Oracle test failure

The only problem for Oracle was the test, which tested nullity on
text/char fields -- but Oracle interprets_empty_strings_as_null.

Backport of d5a0acc from master

comment:17 Changed 4 years ago by Shai Berger

Well, for some reason the commit hook missed it on master, but just to clarify,
d5a0acc fixed the problem there.

comment:18 Changed 4 years ago by Claude Paroz

Thanks Shai!

Note: See TracTickets for help on using tickets.
Back to Top