? ? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
7 Mar 2021

Pull Request for #28501 (comment) .

Summary of Changes

This pull request (PR) extends the fix from #28501 for the database schema check to integer data types other than int and tinyint.

It only relevant for MySQL and MariaDB databases, ie not PostgreSQL.

As stated in PR #28501 and issue #32542 , the display width modifier has no impact on value range or storage size of a database column of one of the integer data types.

While MySQL deprecated these display widths beginning with version 8.0.17 and doesn't include them anymore in the type returned by a SHOW COLUMNS statement, MariaDB still supports and uses display widths modifiers for integer types so it still behaves like MySQL <= 5.7.

With this PR I took the chance for refactoring the parts which have been modified before with #28501 for int and tinyint types, so that code is more readable and also more precise with checking the types and so doesn't hide anymore typos in update SQL scripts.

Testing Instructions

Requirement: Have an installation of current staging or latest 3.9.x nightly build which uses a MySQL or MariaDB database.

Testers please report back which of the following 2 kinds of databases you have used for the test:

  • MySQL database servers with versions lower than 8.0.17 or on any server version of MariaDB
  • MySQL database servers with version 8.0.17 or later

If you have both scenarios available, please test both.

Step 1: Add 2 new update SQL scripts as follows to folder administrator/components/com_admin/sql/updates/mysql:

If you are not comfortable with editing you can also download them using the link in the script names.

CREATE TABLE `#__test_table_1` (
  `id`              int NOT NULL AUTO_INCREMENT,
  `col_int`         int,
  `col_int_u`       int unsigned,
  `col_tinyint`     tinyint,
  `col_tinyint_u`   tinyint unsigned,
  `col_smallint`    smallint,
  `col_smallint_u`  smallint unsigned,
  `col_mediumint`   mediumint,
  `col_mediumint_u` mediumint unsigned,
  `col_bigint`      bigint,
  `col_bigint_u`    bigint unsigned,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE=utf8mb4_unicode_ci;

CREATE TABLE `#__test_table_2` (
  `id`              int(10) NOT NULL AUTO_INCREMENT,
  `col_int`         int(9),
  `col_int_u`       int(9) unsigned,
  `col_tinyint`     tinyint(2),
  `col_tinyint_u`   tinyint(2) unsigned,
  `col_smallint`    smallint(4),
  `col_smallint_u`  smallint(4) unsigned,
  `col_mediumint`   mediumint(6),
  `col_mediumint_u` mediumint(6) unsigned,
  `col_bigint`      bigint(11),
  `col_bigint_u`    bigint(11) unsigned,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE=utf8mb4_unicode_ci;
ALTER TABLE `#__test_table_1` MODIFY `col_int` int DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_int_u` int unsigned DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_tinyint` tinyint DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_tinyint_u` tinyint unsigned DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_smallint` smallint DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_smallint_u` smallint unsigned DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_mediumint` mediumint DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_mediumint_u` mediumint unsigned DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_bigint` bigint DEFAULT 0;
ALTER TABLE `#__test_table_1` MODIFY `col_bigint_u` bigint unsigned DEFAULT 0;

ALTER TABLE `#__test_table_2` MODIFY `col_int` int(9) DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_int_u` int(9) unsigned DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_tinyint` tinyint(2) DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_tinyint_u` tinyint(2) unsigned DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_smallint` smallint(4) DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_smallint_u` smallint(4) unsigned DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_mediumint` mediumint(6) DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_mediumint_u` mediumint(6) unsigned DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_bigint` bigint(11) DEFAULT 0;
ALTER TABLE `#__test_table_2` MODIFY `col_bigint_u` bigint(11) unsigned DEFAULT 0;

The reason why it needs the 2nd script is that the database schema checker checks only for the existence of the table for CREATE TABLE statements but not for the columns. This is checked eg for ALTER TABLE ... MODIFY statements which here just add a default value for having some change but don't touch the data type.

Step 2: Go to "Extensions -> Manage Database".

Result: An error alert about the 2 tables missing is shown, and some 23 database problems have been found (the exact number may differ, but it doesn't matter for this test step here):
step-1_mysql-8

Step 3: Close the red error alert by using the "x" button at its top right corner (only visible on hover).

Step 4: Use the "Fix" button.

Result: See section "Actual result BEFORE applying this Pull Request" below.

Step 5: Edit the 2nd SQL script 3.9.26-2021-03-06.sql and apply a typo by appending something to the data type for the database table for which no database problem is shown, ie:

  • For MySQL database with versions lower than 8.0.17 or on any server version of MariaDB chose #__test_table_2
  • For MySQL database with version 8.0.17 or later chose #__test_table_1

See example here with an X appended to the int type of the col_int column:

ALTER TABLE `#__test_table_2` MODIFY `col_int` intX DEFAULT 0;

Then go again to "Extensions -> Manage Database", or if still there reload the page.

Result: There is still no database problem shown for that table, i.e. the typo is silently ignored.

Step 6: Revert the change with the typo made in the previous step in the 2nd SQL script.

Step 7: Only for MySQL database servers with versions lower than 8.0.17 or on any server version of MariaDB.

If you have a MySQL database servers with version 8.0.17 or later, skip this test step.

This test step doesn't test this PR but proofs that display widths don't matter for the value range and so safely can be ignored.

Using a tool like eg phpMyAdmin, insert a row as follows into the 2nd table #__test_table_2:

INSERT INTO `#__test_table_2` (`col_int`, `col_int_u`, `col_tinyint`, `col_tinyint_u`, `col_smallint`, `col_smallint_u`, `col_mediumint`, `col_mediumint_u`, `col_bigint`, `col_bigint_u`)
     VALUES (2147483647, 4294967295, 127, 255, 32767, 65535, 8388607, 16777215, 999999999999, 999999999999);

Replace #__ with your table prefix before executing the statement.

Verify that the values of all columns have been saved for that record with the values specified in the insert statement, regardless of the fact that they all exceed the display width of the particular column.

Step 8: Apply the changes from this PR.

Step 9: Go again to "Extensions -> Manage Database", or if still there reload the page.

Result: See section "Expected result AFTER applying this Pull Request" below.

Step 10: Repeat Step 5: with the typo having something appended to the data type for any column of any of the 2 tables.

Result: A database problem is shown due to the not matching type with the typo, ie the typo in the SQL script becomes obvious now. Don't use the "Fix" button now, it would cause an SQL error.

Actual result BEFORE applying this Pull Request

  • On MySQL database servers with versions lower than 8.0.17 or on any server version of MariaDB:
    6 errors reported for #__test_table_1, no errors reported for #__test_table_2.
    step-2_mysql-5-or-mariadb
  • On MySQL database servers with version 8.0.17 or later:
    6 errors reported for #__test_table_2, no errors reported for #__test_table_1.
    step-2_mysql-8
  • Certain typos in update SQL scripts are silently ignored.

Expected result AFTER applying this Pull Request

  • On all versions of MySQL or MariaDB databases:
    No database problems found.
    step-3
  • The check for the data types is precise now, so typos will not be ignored.

Additional info for maintainers / release leads

This PR should go up to 3.10-dev and 4.0-dev, too.

It's most urgent for 4.0-dev since in J4 the database checker is also used for extensions and non only the core, so the likelihood to get a relevant SQL statement in an update SQL script which triggers the error (false database problems found) is much higher than in 3.9.x/3.10.

Documentation Changes Required

None.

avatar richard67 richard67 - open - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Mar 2021
Category Libraries
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
Labels Added: ?
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
Title
[3.x] [WiP] Make database schema check ignore display widths for all integer types on MySQL or MariaDB databases
[3.x] Make database schema check ignore display widths for all integer types on MySQL or MariaDB databases
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar richard67 richard67 - change - 7 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 7 Mar 2021
avatar brianteeman
brianteeman - comment - 9 Mar 2021

Following your detailed instructions

Database Version 5.5.5-10.5.7-MariaDB
Database Collation utf8_general_ci
Database Connection Collation utf8mb4_general_ci
PHP Version 7.3.27

Comments below are on your instructions not on the testing

Step 2
I have 22 problems not 23. No message about the database schema not matching

Step 4
Still get the error message but clearly the tables have been created or the problems would not be reported
image

Leaving the page and then revisiting it and the Error is not longer displayed - just the Warning

Step 5

Replace #__ with your table prefix before executing the statement.

Really? I'm assuming that's a mistake. But tested with replacement and without

avatar brianteeman brianteeman - test_item - 9 Mar 2021 - Tested successfully
avatar brianteeman
brianteeman - comment - 9 Mar 2021

I have tested this item successfully on ff2a226


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32605.

avatar richard67
richard67 - comment - 9 Mar 2021

@brianteeman

Step 2
I have 22 problems not 23. No message about the database schema not matching

Thanks, will add a hint to testing instructions that exact number of problems doesn't matter for that test step.

Step 4
Still get the error message but clearly the tables have been created or the problems would not be reported

The red error alerts stay on the page until closed, even on reload. That's why step 3 tells to close it. Maybe another bug we found.

Step 5

Replace #__ with your table prefix before executing the statement.
Really? I'm assuming that's a mistake. But tested with replacement and without

You are right, that's a copy&paste remainder. Will remove it.

Thanks for testing and feedback.

avatar richard67 richard67 - change - 9 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 9 Mar 2021
avatar richard67 richard67 - change - 9 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 9 Mar 2021
avatar richard67 richard67 - change - 9 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 9 Mar 2021
avatar richard67 richard67 - change - 9 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 9 Mar 2021
avatar richard67 richard67 - change - 9 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 9 Mar 2021
avatar richard67 richard67 - change - 9 Mar 2021
The description was changed
avatar richard67 richard67 - edited - 9 Mar 2021
avatar alikon alikon - test_item - 10 Mar 2021 - Tested successfully
avatar alikon
alikon - comment - 10 Mar 2021

I have tested this item successfully on ff2a226

with MySQL 8.0.22


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32605.

avatar alikon alikon - change - 10 Mar 2021
Status Pending Ready to Commit
avatar alikon
alikon - comment - 10 Mar 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32605.

avatar rdeutz rdeutz - change - 10 Mar 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-03-10 11:39:23
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 10 Mar 2021
avatar rdeutz rdeutz - merge - 10 Mar 2021

Add a Comment

Login with GitHub to post a comment