Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#27859 closed Bug (fixed)

Migration to create TextField with db_index=True crashes on MySQL

Reported by: Daniel Quinn Owned by: Mariusz Felisiak
Component: Migrations Version: dev
Severity: Normal Keywords: mysql oracle index
Cc: zubair.alam.jmi@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a GPL project that uses Django as its base that typically works using SQLite. However, some of my users have been trying to use it with MySQL and this has presented a strange annoyance.

In one of my models, I have this line:

content = models.TextField(db_index=True)

this generates a migration that looks like this:

...
('content', models.TextField(db_index=True)),
...

...which seems reasonable enough to me. However, while this works just fine in SQLite and PostgreSQL, users attempting to install the project using MySQL were treated to this explosion when they ran manage.py migrate:

django.db.utils.OperationalError: (1170, "BLOB/TEXT column 'content' used in key specification without a key length")

Now understandably, if you're writing a project *for* MySQL, you wouldn't put an index on a TextField because you know that MySQL doesn't support that, but in the interests of portability, I would think a warning, coupled with ignoring the index specification would be more appropriate.

For now, I've just had to rewrite the migration to look like this:

('content', models.TextField(db_index=("mysql" not in settings.DATABASES["default"]["ENGINE"]))),

I'm not proud of that one, but it works for now :-)

There's a longer discussion about this issue on my project's GitHub issue page here: https://github.com/danielquinn/paperless/issues/183 if you're interested.

https://github.com/danielquinn/paperless/issues/183#issuecomment-280863310

Change History (14)

comment:1 by Simon Charette, 8 years ago

Triage Stage: UnreviewedAccepted
Version: 1.10master

Related to #2495.

I also think the correct way of handling that would be to make MySQL's schema editor ignores db_index on TextField.

comment:2 by Zubair Alam, 8 years ago

Owner: changed from nobody to Zubair Alam
Status: newassigned

comment:3 by Tim Graham, 8 years ago

Keywords: mysql added; MySQL removed
Summary: makemigrations creates migrations that don't work in MySQLMigration to create TextField with db_index=True crashes on MySQL
Type: UncategorizedBug

comment:4 by Zubair Alam, 8 years ago

Cc: zubair.alam.jmi@… added
Has patch: set
Last edited 8 years ago by Tim Graham (previous) (diff)

comment:5 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:6 by Zubair Alam, 8 years ago

Patch needs improvement: unset

comment:7 by Mariusz Felisiak, 8 years ago

Patch needs improvement: set

comment:8 by Zubair Alam, 8 years ago

Owner: Zubair Alam removed
Status: assignednew

comment:9 by Mariusz Felisiak, 8 years ago

Owner: set to Mariusz Felisiak
Status: newassigned

comment:10 by Mariusz Felisiak, 8 years ago

Keywords: oracle index added
Patch needs improvement: unset

comment:11 by GitHub <noreply@…>, 8 years ago

In 5cff2cb4:

Refs #27859 -- Refactored BaseDatabaseValidation to use check_field_type().

Thanks Tim Graham for the review.

comment:12 by Tim Graham <timograham@…>, 7 years ago

In b3eb6eaf:

Refs #27859 -- Added DatabaseWrapper.display_name.

Thanks Tim Graham for the review.

comment:13 by GitHub <noreply@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 538bf434:

Fixed #27859 -- Ignored db_index for TextField/BinaryField on Oracle and MySQL.

Thanks Zubair Alam for the initial patch and Tim Graham for the review.

comment:14 by Claude Paroz, 7 years ago

I have a PR that partially reverts this fix and allows for TextField.db_index for MySQL. Thoughts?

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