Opened 5 years ago

Closed 5 years ago

#31144 closed Bug (fixed)

MySQL unique constraints incorrectly limited to 255 char when it should be 1000.

Reported by: Steven Mapes Owned by: Rohit Jha
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: MySQL, MariaDB, Indexes, Unique Keys
Cc: Claude Paroz, Adam Johnson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Steven Mapes)

There is a bug within db.backends.mysql.validation.check_field_type where the maximum unique constraints is still being limited to 255 characters even though MySQL supports unique keys of up to 1000 characters by default and InnoDB supports 3072 bytes for InnoDB tables that use DYNAMIC or COMPRESSED row format and 767 bytes for InnoDB tables that use the REDUNDANT or COMPACT row format.

MySQL supports VARCHARs of up to 65,535 length. It's only CHAR that is restricted to 255 and as Django creates VARVCHARs there is no need to enforce that limit.

Reference -

  1. InnoDB - https://dev.mysql.com/doc/refman/8.0/en/innodb-limits.html
  2. MyISAM - https://dev.mysql.com/doc/refman/8.0/en/myisam-storage-engine.html
  3. CHAR and VARCHAR Types -https://dev.mysql.com/doc/refman/8.0/en/char.html

Example

CREATE DATABASE testCHARACTER SET utf8 COLLATE utf8_general_ci;
USE test;

CREATE TABLE `example_innodb` (
  `example1` VARCHAR(1024) DEFAULT NULL,
  `example2` VARCHAR(1024) DEFAULT NULL,
  UNIQUE KEY `UQ_example1` (`example1`),
  UNIQUE KEY `UQ_example2` (`example2`)
) ENGINE=INNODB DEFAULT CHARSET=utf8

DROP TABLE example_innodb;
DROP DATABASE test;

It just needs the max length to be increased on line 38 of db.backends.mysql.validation

This is across all branches

Change History (18)

comment:1 by Steven Mapes, 5 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 5 years ago

Cc: Claude Paroz added
Resolution: wontfix
Status: newclosed
Summary: MySQL unique constraints incorrectly limited to 255 char when it should be 1000MySQL unique constraints incorrectly limited to 255 char when it should be 1000.
Version: 3.0master

Thanks for this ticket, however Django supports MySQL 5.6+. Moreover this restriction depends on character set, database version, database engine, and parameters (see related ticket #28661), so it's quite complicated to determine the real upper bound. Even with MySQL 8.0 we still have the following restriction:

InnoDB tables that use DYNAMIC or COMPRESSED row format and 767 bytes for InnoDB tables that use the REDUNDANT or COMPACT row format.

which means 255 characters with utf8 and 191 characters with utf8mb4.

comment:3 by Mariusz Felisiak, 5 years ago

Cc: Adam Johnson added

comment:4 by Steven Mapes, 5 years ago

So why not raise a warning or, better yet, let the database itself handle whether or not there is an error applying the index and thus the migration will not apply anyway just as if you use an index_together or unique_together that is over the max length. These are handled by MySQL as Django lets them pass and is a work around on the unique=True issue.

comment:5 by Adam Johnson, 5 years ago

Resolution: wontfix
Status: closednew

I agree with Steven here. MariaDB also ships many storage engines other than InnoDB with different limits. It might be better to "translate" errors coming out of the database to add a bit of django context e.g. which model is causing the problem?

comment:6 by Mariusz Felisiak, 5 years ago

I'm not a MySQL expert, but I think that MySQL doesn't raise any error it'll just create an unique constraint for the first e.g. 191 characters and ignores the remaining chars. So users will not be aware that DB doesn't protect uniqueness.

in reply to:  6 comment:7 by Steven Mapes, 5 years ago

Replying to felixxm:

I'm not a MySQL expert, but I think that MySQL doesn't raise any error it'll just create an unique constraint for the first e.g. 191 characters and ignores the remaining chars. So users will not be aware that DB doesn't protect uniqueness.

As a longtime certificate MySQL Dev I can assure you that both MySQL and MariaDB both do not act in this way and will raise error code 1071 (E.G "Specified key was too long; max key length is 3072 bytes") if you try to create an index that is too large.

Here is an example of creating indexes on a UTF-8 table that will not error

CREATE DATABASE djangotest CHARACTER SET utf8 COLLATE utf8_general_ci;
USE djangotest;

CREATE TABLE `example_innodb` (
  `example1` VARCHAR(1024) DEFAULT NULL,
  `example2` VARCHAR(1024) DEFAULT NULL,
  UNIQUE KEY `UQ_example1` (`example1`),
  UNIQUE KEY `UQ_example2` (`example2`)
) ENGINE=INNODB DEFAULT CHARSET=utf8;

DROP TABLE example_innodb;
DROP DATABASE djangotest;

Here is an example of one where the Error Code: 1071 will be triggered

CREATE DATABASE djangotest2 CHARACTER SET utf8 COLLATE utf8_general_ci;
USE djangotest2;

CREATE TABLE `example_innodb` (
  `example1` VARCHAR(2048) DEFAULT NULL,
  `example2` VARCHAR(2048) DEFAULT NULL,
  UNIQUE KEY `UQ_example1` (`example1`),
  UNIQUE KEY `UQ_example2` (`example2`)
) ENGINE=INNODB DEFAULT CHARSET=utf8;

DROP TABLE example_innodb;
DROP DATABASE djangotest2;

For this you will receive the following error:

Error Code: 1071
Specified key was too long; max key length is 3072 bytes

Here is a 3rd example showing an UTF8MB4 table

CREATE DATABASE `example3`CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci; 
USE `example3`;
CREATE TABLE `example3`.`example` ( `column1` VARCHAR(3072) ) ENGINE=INNODB CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; 
ALTER TABLE `example3`.`example`  ADD  UNIQUE INDEX `test_unique` (`column1`(3072));

This will again generate the an Error Code: 1071 - Specified key was too long; max key length is 3072 bytes

Here is an example of the maximum UTF8MB4 index being created

ALTER TABLE `example3`.`example`  ADD  UNIQUE INDEX `test_unique2` (`column1`(768));

Then simple try to index one more character and you will be over the byte length

ALTER TABLE `example3`.`example`  ADD  UNIQUE INDEX `test_unique2` (`column1`(769));

So the Error Code: 1071 will trigger again

comment:8 by Steven Mapes, 5 years ago

Easy pickings: set
Keywords: MariaDB Indexes Keys added; MAriaDB removed

comment:9 by Sameeran Bandishti, 5 years ago

Hi! I would like to take up this issue if no one is already working on it. I'm new to this codebase, so I might need a little help if that's okay :)

in reply to:  9 comment:10 by Steven Mapes, 5 years ago

Replying to Sameeran Bandishti:

Hi! I would like to take up this issue if no one is already working on it. I'm new to this codebase, so I might need a little help if that's okay :)

That would be great from my point of view. I would just make the change and publish a PR for it myself but I'm unsure on how best to write the unit tests without them being actual integration tests other than using mocks to raise the error or not but then it seems rather pointless of a test.

For me the change is either

  1. Remove lines 37 through 45 of db.backends.mysql.validation.check_field_type as they are simply not needed as the DB itself will trigger an error.
  2. Change checks.Error on line 40 to checks.Warning and update the error message to be one that is more useful. E.G to instruct the developer to check the documentation and for the limits based on the character set they are using for the database/table/column.

The docs should also be updated to remove any reference to MySQL having a 255 char limit on VARCHAR as its not true

comment:11 by Mariusz Felisiak, 5 years ago

Easy pickings: unset

That's definitely not an "easy picking".

comment:12 by Adam Johnson, 5 years ago

Change checks.Error on line 40 to checks.Warning and update the error message to be one that is more useful. E.G to instruct the developer to check the documentation and for the limits based on the character set they are using for the database/table/column.

Given what I currently know of general ecosystem and which table types are ore common, I think it's probably more favourable to move to a warning than remove the check entirely.

The docs should also be updated to remove any reference to MySQL having a 255 char limit on VARCHAR as its not true

I think it's still useful to mention briefly as a potential stumbling block, with links to MySQL and MariaDB docs around the issue.

comment:13 by Adam Johnson, 5 years ago

Triage Stage: UnreviewedAccepted

comment:14 by Ashutosh Chauhan, 5 years ago

I would like to try to do it.

comment:15 by Rohit Jha, 5 years ago

Owner: changed from nobody to Rohit Jha
Status: newassigned

comment:16 by Mariusz Felisiak, 5 years ago

Has patch: set
Needs tests: set
Patch needs improvement: set

comment:17 by Mariusz Felisiak, 5 years ago

Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:18 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 2695ac8e:

Fixed #31144 -- Relaxed system check for max_length of CharFields on MySQL/MariaDB by turning into a warning.

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