Opened 9 years ago
Closed 8 years ago
#35958 closed defect (bug) (fixed)
Failing dbDelta tests due to MySQL error
Reported by: | johnbillion | Owned by: | jeremyfelt |
---|---|---|---|
Milestone: | 4.8 | Priority: | normal |
Severity: | normal | Version: | 4.2 |
Component: | Database | Keywords: | has-patch |
Focuses: | Cc: |
Description
The Tests_dbDelta
tests are failing on my local test setup (they have been for a while now, and I had been ignoring them due to assuming I'd messed something up).
The culprit for the failures is this MySQL error when creating the test table in Tests_dbDelta::setUp()
:
#1071 - Specified key was too long; max key length is 1000 bytes
The query looks like this:
CREATE TABLE wptests_dbdelta_test ( id bigint(20) NOT NULL AUTO_INCREMENT, column_1 varchar(255) NOT NULL, PRIMARY KEY (id), KEY key_1 (column_1), KEY compound_key (id,column_1), FULLTEXT KEY fulltext_key (column_1) ) ENGINE=MyISAM
Is this an environment problem or is there a bug in the tests which doesn't take into account this key length limit?
Environment:
- MySQL server: 5.5.44
- MySQL client: 50624 (5.6.24)
- PHP: 5.6.99-hhvm
- Web server: Nginx 1.9.5
- OS: OS X 10.10.3
Attachments (4)
Change History (20)
#2
@
9 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
Confirmed, happens when you have database with a utf8mb4
collation.
This ticket was mentioned in Slack in #core by ocean90. View the logs.
8 years ago
#4
@
8 years ago
Here's a suggested patch for this ticket. I couldn't see a good reason in the tests for the column length to be any particular byte length, so I just chose 100 because it's a common length.
#6
@
8 years ago
I can confirm this on a new VVV instance with MariaDB 5.5. A similar thread is here https://core.trac.wordpress.org/ticket/35249
Since WP 4.6, all tables are set with utf8mb4
charset and collation if it is available.
https://core.trac.wordpress.org/browser/tags/4.6/src/wp-includes/wp-db.php#L758
The magic number should be 191
as that is the amount of four-byte characters available under the 767 ENGINE=MyISAM
limit (191 * 4 = 764 bytes)
As string literal:
https://dl.dropboxusercontent.com/s/x30kpsxv5nzyoko/2017-02-24%20at%2018.10.png
As object property:
https://dl.dropboxusercontent.com/s/2apntbcurpwwn74/2017-02-24%20at%2018.16.png
Attached are two patches, one with the 191
string literal in every location, the other with it set as an Object Property. :)
#7
@
8 years ago
I've rebuilt my VVV yesterday and getting these errors now :) If I'm not wrong, VVV changed to MariaDB on January, using utf8mb4
by default. If so, I'm guessing this is going to happen on every new install? /cc @jeremyfelt
#8
@
8 years ago
- Keywords dev-feedback removed
- Milestone changed from Future Release to 4.7.4
- Version set to 4.2
This will likely be even more visible when VVV 2.0 ships, as MariaDB 10.1 is default.
We changed the default core varchar
keys to varchar(191)
in [31349] so that utf8mb4 could be supported. I think it makes sense to change any keys created in the dbDelta
tests as well. It's probably not core's responsibility to check whether a DB can support any more than that.
35958-varchar-191-object-property.diff seems to make the most sense at first glance, though I wouldn't be opposed to 35958-varchar-191-string-literal.diff either.
#9
@
8 years ago
I vote for the object property patch. I think it's clearer to put up top what the purpose of that magic number is, so it's not buried in a SQL query in two separate places.
This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
8 years ago
#14
@
8 years ago
I too prefer the object property patch. I tested that patch locally using VVV 2.0 running MariaDB 10.1, and the unit tests no longer throw errors for me.
Output before: https://gist.github.com/desrosj/4c4f2b3a2555c1ee6030f4d4ae153339
#15
@
8 years ago
35958.diff applies the maximum index length to the key definitions rather than the field definitions, which matches the approach taken in [31349] when core was adjusted to better support utf8mb4. VARCHAR will support larger fields, the indexes have the max size.
Many of these are not entirely necessary, as the tests are using dbDelta()
to determine that nothing changed and don't actually end up issuing MySQL commands. But, for consistency, I updated all of the key definitions that would require the max length to be set.
I think that this is partly an issue with the tests, since a key on a
VARCHAR(255)
field can only be191
in length, when using utf8mb4. So I think that the keys ought to look like this:However, I don't get this error when I run the tests on my local machine. I believe that the reason is likely due to a difference in MySQL configuration, probably your MySQL database has a different default character set than mine. This is why WordPress always specifies the character set and collation when creating tables. But we don't do that here in these unit tests, which is likely why we're seeing inconsistent failures.
I'm no expert though, so I'm happy to be corrected on all this. :-)