Opened 21 months ago
Last modified 37 hours ago
#49364 new defect (bug)
dbDelta() should not change display width for integer data types on MySQL 8.0.17+
Reported by: | SergeyBiryukov | Owned by: | |
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Database | Keywords: | has-patch |
Focuses: | Cc: |
Description
Background: #49344
MySQL 8.0.17 removed support for the display width attribute for integer data types:
As of MySQL 8.0.17, the ZEROFILL attribute is deprecated for numeric data types, as is the display width attribute for integer data types. Support for ZEROFILL and display widths for integer data types will be removed in a future MySQL version. Consider using an alternative means of producing the effect of these attributes.
With this change, when creating a table, BIGINT(20)
becomes just BIGINT
, causing quite a few failures in tests/dbdelta.php
(25 in total). They all look similar to this:
2) Tests_dbDelta::test_column_type_change Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( - 'wptests_dbdelta_test.id' => 'Changed type of wptests_dbdelta_test.id from bigint(20) to int(11)' + 'wptests_dbdelta_test.id' => 'Changed type of wptests_dbdelta_test.id from bigint to int(11)' ) tests/phpunit/tests/dbdelta.php:151
Most of these failures will be addressed in #49344.
This ticket is for addressing the remaining failure caused by the same BIGINT(20)
/BIGINT
discrepancy coming from wp_get_db_schema()
:
1) Tests_dbDelta::test_wp_get_db_schema_does_no_alter_queries_on_existing_install Failed asserting that an array is empty. tests/phpunit/tests/dbdelta.php:693
Currently, when running on MySQL 8.0.17+, dbDelta()
tries to convert BIGINT
back to BIGINT(20)
, INT
back to INT(11)
, etc. This does not have any effect on the database, but is pointless and should not happen.
Attachments (1)
Change History (10)
This ticket was mentioned in Slack in #core by sergey. View the logs.
14 months ago
#5
@
5 months ago
The original ticket states:
Currently, when running on MySQL 8.0.17+, dbDelta() tries to convert BIGINT back to BIGINT(20), INT back to INT(11), etc. This does not have any effect on the database, but is pointless and should not happen.
However, this isn't always a silent failure. It can cause failures where an INT with a display width is also a primary key. Consider the plugin below:
<?php /* * Plugin Name: dbDelta Display Width Issue * Description: dbDelta Display Width Issue * Version: 1.0.0 */ if ( ! defined( 'ABSPATH' ) ) { exit; // Exit if accessed directly } function ddwi_install() { require_once ABSPATH . 'wp-admin/includes/upgrade.php'; dbDelta( <<<SQL CREATE TABLE `wp_ddwi_test` ( `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY, ) SQL ); } register_activation_hook( __FILE__, 'ddwi_install' );
The first activation will go without issues. However deactivating and re-activating the plugin attempts to run the following (unnecessary) alter statement:
ALTER TABLE wp_ddwi_test CHANGE COLUMN `id` `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY
This yields the following error:
WordPress database error Multiple primary key defined for query ALTER TABLE wp_ddwi_test CHANGE COLUMN `id` `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT PRIMARY KEY
and gives an error on WP-CLI , or through online activation about excess output produced during activation.
#6
@
4 months ago
- Milestone changed from Future Release to 5.9
I'd like to solve the MySQL 8 test failures for 5.9 to that we can look at expanding our testing combinations.
This ticket was mentioned in PR #1781 on WordPress/wordpress-develop by pbearne.
37 hours ago
- Keywords has-unit-tests added
Add regx to dbDelta() function so any calls in the old form are handled
Trac ticket: https://core.trac.wordpress.org/ticket/49364
This ticket was mentioned in PR #1782 on WordPress/wordpress-develop by pbearne.
37 hours ago
Add regex to dbDelta() function so any calls in the old form are handled
and included the corrected schema
Trac ticket: https://core.trac.wordpress.org/ticket/49364
#9
@
37 hours ago
- Keywords needs-testing has-unit-tests removed
Hi All
I dug into this and checked the testing
It seems to me that we will have issues with current code/plugins still trying to use the old format
So I added a regx_replace to fix the SQL on the fly
Maybe we should look at adding a do_it_wrong notice but I didn't do that it would increase the overhead
The new patch is in pull request #1781 and the combined patch is in #1782
I hope this helps
Paul
In 47184: