@wilsonge Please test this, it should fix #9156.

I have tested this item successfully on 58d090d

Before Patch:
92 database changes were checked successfully.

After patch:
101 database changes were checked and found 8 issue.

After Fix:
101 database changes were checked successfully.

Tested With: utf8mb4_general_ci

Yeah, that's how it is expected to work, thanks @810 for the test:

• Depending on the utf8mb4 support of mysql database server and mysql related database client and on history of not applied past database fixes, there may still be database issues found after updating.

• But the "Fix" button will fix these now with my PR, which has failed before in some cases, especially if no utf8mb4 support.

• If no utf8mb4 support it can also happen that the "Fix" button is not enough, people also have to discover the new 3 plugins and install them and then activate them.

• But at the end all should be fine.

• I.e. the remaning problems and how to fix them should be added to the release notes, known issues, if there is no other way to fix them.

On a other test site i get:
Specified key was too long; max key length is 767 bytes SQL=ALTER TABLE #__menu MODIFY alias varchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL COMMENT 'The SEF alias of the menu item.';

@810 yes that's the case when no utf8mb4 support. But "Fix" for the database issues (should be more than with the other test before) and then "discover" for new extenions and install the 3 found new plugins then should solve this. This PR just makes it possible that "Fix" works in this case.

ok now its working, i removed that line, and click fix, after all checks i added :

 ALTER TABLE `#__menu` MODIFY `alias` varchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL COMMENT 'The SEF alias of the menu item.';

back, and did the fix again, now it was correct.

So maybe the menu fix, must be the last item

@810

So maybe the menu fix, must be the last item

Am not sure if this will be the solution, maybe others have problems with other tables? From reading the code I have no explanation why this should help.

Maybe you can redo your test from start with same conditions as before, but this time using a modified 3.5.0-2015-07-01.sql file with your suggested fix, and let us know if that worked for you?

If you want you can set your test result back to not tested or even to not successful, I leave this up to you.

Meanwhile I hope @wilsonge can test and let me know what he found out.

This has not solved the issue here with menu

 Specified key was too long; max key length is 767 bytes SQL=ALTER TABLE `#__menu` MODIFY `alias` varchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL COMMENT 'The SEF alias of the menu item.'; 

I can only solve it the way I proposed earlier i.e. by adding DEFAULT
Change
ALTER TABLE#__menuMODIFYaliasvarchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL COMMENT 'The SEF alias of the menu item.';

to
ALTER TABLE#__menuMODIFYaliasvarchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '' COMMENT 'The SEF alias of the menu item.';

@infograf768 Thanks for the feedback. I will check later, just got up and have a few things to do before that.
I am not sure if an empty string as default value is what we want for a not null column, but the info might help to find a way around.

This PR has received new commits.

CC: @810

I corrected a detail in database schema manager handling for alter table with drop and add index and will do forther corrections in a while, please be patient and wait further changes within the next 3 to 4 hours.

@810 @infograf768 I have not finished everything, but if you find time and mood you could test if my latest changes make the "Fix" button for the database problems work without having to modify any of the SQL files.

You can find an update container here: http://test5.richard-fath.de/Joomla_3.5.0-beta2-Beta-Update_Package_test1.zip.

Or you can use Joomla Update with following custom URL (note it is http, not use https): http://test5.richard-fath.de/list_test4.xml.

This PR has received new commits.

CC: @810

I have tested this item successfully on 74d5711

@810 Thanks for the test. Just to be sure I know all: Did you test both databases you tested before? And did it work this time with Fix Button and maybe also Discover Extension but without having to temporarily modify an SQL file?

@infograf768 @wilsonge Can you test it now, too? I am not sure if all is corrected now, but it should be some step forward with some progress.

@infograf768 I followed your suggestion with the default value. Am curious if this will change anything.

Did tested on the 2 databases, both successfull. Just with the fix button.

•101 database changes were checked successfully.
•211 database changes did not alter table structure and were skipped.

@810 Thanks for the info and for testing.

Updated detailed description and test instructions

@infograf768 @wilsonge PR is ready for test now, updated instructions see at the top.

Other remaining problems we can solve with another PR if this one here makes the fixing of reported database problems work.

please look at @mbabker comment though
fcb6d90#commitcomment-16149922

@infograf768 Hmm, then I might roll this one little change back later. But could you test anyway and let me know the results?

The DEFAULT '' fix is just a symptom of a bigger problem we haven't worked out yet (i mean it would appear that on JM's install the key being wrong means that statement that creates the key hasn't run properly)... (hence the default hack/fix works before and after) :/ There's no way it should be part of this PR

OK, I will revert now the default for the menu table's alias column.
@infograf768 @810 Can you test again after the next commit when travis is OK?

This PR has received new commits.

CC: @810

@infograf768 @810 If you wanna test again and use the update container links I provided above in the test instructions, please wait until I have updated the container. I'll let you know here when I have updated it.

I have tested this item successfully on 2c0cd55

@infograf768 @mbabker Regarding whether the alias column of the menu table should have a default value or not I have found a few places where things make me believe it could be ok to have an empty string as default.

I come from Oracle, where are real not null columns.

But Mysql seems to use internal defaults depending on strict mode or not strict mode, a crap from my point of view.

See http://stackoverflow.com/questions/8558260/what-is-the-default-value-for-a-field-if-no-default-value-is-provided

Or http://dev.mysql.com/doc/refman/5.7/en/data-type-defaults.html

So maybe it was OK to have the default?

Please check and discuss and let me know the result so I can add it back to this PT if OK.

From a purely database perspective, it's probably OK to have an empty default. Again though, that is totally scope isolated and not contextual to the full application state. If the application cannot have empty aliases and function properly, the database schema should not allow it, and IMO the database schema should mirror the application's data requirements. I don't know what the truth of that assumption is (whether the code breaks on an empty alias condition or not), but bigger picture that's what I'm looking at personally.

The PHP forbids an empty alias field.... (as in they autopopulate it from the title if left empty). So it definitely should be required. And again how does setting a default key fix a too long index? Until someone can explain that to me, it's a hack for an issue we don't fully understand yet

From data point of fiew you are right, Michael, and with my Oracle databases I pretty much could rely on a column with a not null contraint and to default value would be rejected on insert or update.

But all I found when I googled for "mysql" and "not null" and "default none" or so was that in case of mysql you cannot rely on that, and that some mysql versions even required a default value to be specified when a not null constraint is on a column, but what I found was not 100% reliable in my opinion so I not post it here (was mainly discussions on stackexchange).

@wilsonge Maybe a mysql bug even?

But @wilsonge If you can find some time thest this PR as it is so we see that other problems have disappeared and the db schema manager works well now.

But all I found when I googled for "mysql" and "not null" and "default none" or so was that in case of mysql you cannot rely on that, and that some mysql versions even required a default value to be specified when a not null constraint is on a column

:facepalm:

Mysql has always had an issue with null values.
On 20 Feb 2016 8:38 pm, "Michael Babker" notifications@github.com wrote:

But all I found when I googled for "mysql" and "not null" and "default
none" or so was that in case of mysql you cannot rely on that, and that
some mysql versions even required a default value to be specified when a
not null constraint is on a column

:facepalm:

—
Reply to this email directly or view it on GitHub
#9170 (comment).

And what can we do now?
Maybe we could redefine (modify) the column without the not null constraint and no default, and then add the not null constraint again, all without the character set and collation conversions, and those do then in a 3rd step?

My problem is that I cannot replicate the problem with that 1 alter table statement for the alias column of the menu table.
The Fix of the dabase problems worked with and without the default value for me, on both mysql 5.1.73 and 5.5.46, using the mysqli driver.

Two tests:
Test 1

clean install of 3.4.8
67 tables (all utf8_general_ci)

Other Information:

Database schema version (in #__schemas): 3.4.0-2015-02-26.
Update version (in #__extensions): 3.4.8.
Database driver: mysqli.
79 database changes were checked successfully.
142 database changes did not alter table structure and were skipped.

uploaded Joomla_3.5.0-beta2-Beta-Update_Package_test1.zip (manual unzip and upload)
67 tables (all utf8_general_ci)

8 Database Problems Found:

Database schema version (3.4.0-2015-02-26) does not match CMS version (3.5.0-2015-11-05).
Database update version (3.4.8) does not match CMS version (3.5.0-beta2).
Table 'zyexs_session' does not have column 'session_id' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_user_keys' does not have column 'series' with type 'varchar(191)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_banners' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_categories' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_contact_details' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_content' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_menu' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_newsfeeds' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_tags' does not have column 'alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_ucm_content' does not have column 'core_alias' with type 'varchar(400)'. (From file 3.5.0-2015-07-01.sql.)
Table 'zyexs_contentitem_tag_map' should not have index 'idx_tag'. (From file 3.5.0-2015-10-26.sql.)
Table 'zyexs_contentitem_tag_map' should not have index 'idx_type'. (From file 3.5.0-2015-10-26.sql.)

Other Information:

Database schema version (in #__schemas): 3.4.0-2015-02-26.
Update version (in #__extensions): 3.4.8.
Database driver: mysqli.
95 database changes were checked successfully.
211 database changes did not alter table structure and were skipped.

Clicked on FIX resulted in:
1071 Specified key was too long; max key length is 767 bytes SQL=ALTER TABLE#__menuMODIFYaliasvarchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL COMMENT 'The SEF alias of the menu item.';

Server version: 5.6.24 - MySQL Community Server (GPL)
Database client version: libmysql - mysqlnd 5.0.11-dev

Two tests:
Test 2

clean install of 3.4.8
67 tables (all utf8_general_ci)

Other Information:

Database schema version (in #__schemas): 3.4.0-2015-02-26.
Update version (in #__extensions): 3.4.8.
Database driver: mysqli.
79 database changes were checked successfully.
142 database changes did not alter table structure and were skipped.

uploaded Joomla_3.5.0-beta2-Beta-Update_Package_test1.zip (via upload & install joomla extension)
67 tables (23 utf8_general_ci, 44 utf8mb4_general_ci)

1 Database Problem Found:

Table 'werma_user_profiles' does not have column 'profile_value' with type 'TEXT'. (From file 3.3.4-2014-08-03.sql.)

Other Information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta2.
Database driver: mysqli.
100 database changes were checked successfully.
211 database changes did not alter table structure and were skipped.

Clicked on FIX resulted in:
Other Information:

Database schema version (in #__schemas): 3.5.0-2015-11-05.
Update version (in #__extensions): 3.5.0-beta2.
Database driver: mysqli.
101 database changes were checked successfully.
211 database changes did not alter table structure and were skipped.

67 tables (23 utf8_general_ci, 44 utf8mb4_general_ci)

_banner_tracks
_content_frontpage
_finder_links_terms*
_finder_taxonomy_map
_modules_menu
_ucm_base
_update_sites_extensions
_user_usergroup_map

Server version: 5.6.24 - MySQL Community Server (GPL)
Database client version: libmysql - mysqlnd 5.0.11-dev

It seems the Joomla! Updater and the Extension installer work differently regarding SQL update files processing.
I will see what I can find out tomorrow (Sunday), need sleep.
If someone knows more or finds out something before, let me know.
Thanks for testing so far.

It is a well known fact that updating and database->fix work differently. The updater runs each command put in the sql files. The database->fix only processes several known structural sql commands.
As reported many times: running database->fix on a manual updated filesystem with new files is NOT the equivalent of updating. They are two different things!

@sovainfo Yes, this difference between updater or extension installer and the dabaase->fix is clear.

But after having seen @aschkenasy 's 2 tests, both with same database server and client version, I think that when updating via the extension installer, the sql might be processed still by the 3.4.8 driver php, while with Joomla! Updater the sql might be processed by the new updated 3.5.0 driver. Can this be?

His results for the test using the extension installer on a utf8mb4 capable database looks like what I get when usng Joomla! Updater with a not utf8mb4 capable database.

If updating manually, one would have to extract the filesystem first, then make a request in the CMS post-upgrade to click the fix button. You'd be in the 3.5 filesystem. That's actually the same order the update component runs in (extracts all files then loops back into the application to trigger a mock JInstaller routine).

If updating via the Extension Manager (uploading the update ZIP there as you would an extension), the full update will run on a hybrid pre- and post- update state (it all depends on what order things are loaded into memory; infrastructure like the application, database, etc. will all be on pre-update versions).

Well this explains the results above.

@richard67 Sorry, misread your comment. Not aware of different processing of sql files between Joomla! Update and Extension installer.

@mbabker I'd like to come back to the default value discussion, please don't get mad at me.

I agree with you that a good database should care for them being not null and not any empty sting default because the aliasses are required to build URLs.

But this is true not only for menu items but also for articles and categories in order to build URLs to blog and list layouts not having a menu item, and when I check the joomla.sql I see that all these alias columns have a default value of empty string in the create table statements, only the one for the menu table not.

This makes no sense to me, I would expect those alias columns all in the same way regarding default value.

So either we have to remove those defaults everywhere where we not want the alias column to be an empty string or null, or if there once was a reason to use the defaults of which we just are not aware anymore, we have to add it to the menu table's alias column.

Or am I wrong?

@mbabker Another idea regarding the problems when using the extension installer for doing the Joomla! Update:

Is there a way for the extension installer to determine if what he just has unpacked and will install is a Joomla Update or another extension?

If so, could the extension installer then somehow invoke the Joomla! Updater to continue processing the update package?

If this was possible, we would not run into the problem that the database driver running the statemets or the database schema manager to later do fixes if something failed still might be the old ones when using the extension installer to do a Joomla! Update which includes changes on database drivers or so.

So or so, regardless of other problems discussed above, the database schema manager is not able to handle "ALTER TABLE tablemane CONVERT TO CHARACTER SET charset COLLATE collation" statements, and so it will not be able to detect when using the "Fix" button later if these statements had been processed or not (because some other SQL failed during the update).
I think about implementing this.

Is there a way for the extension installer to determine if what he just has unpacked and will install is a Joomla Update or another extension?

No. The Extension Manager doesn't have awareness of what packages it is dealing with and JInstaller libraries should not be coupled to a specific component implementation. This also breaks in a very bad way the currently accepted convention of being able to run core updates in this manner (i.e. in case there is a fatal bug in the update component which keeps people from using it to update it becomes mandatory).

This makes no sense to me, I would expect those alias columns all in the same way regarding default value.

Joomla's consistent at being inconsistent. That's all I'll say on this one

Hmm, ok then. I'll let you know if I have something new which could help on the one or other problem. Please do the same for me.

Sorry to contradict what @mbabker is saying, but it is simply not true!
See libraries/cms/installer/adapter/file.php which is used when you update Joomla using Extension manager->Install.
See protected function setupInstallPaths

if ($this->name == 'files_joomla')
        {
            // If we are updating the Joomla core, set the root path to the root of Joomla
            $this->parent->setPath('extension_root', JPATH_ROOT);
        }
        else
        {
            $this->parent->setPath('extension_root', JPATH_MANIFESTS . '/files/' . $this->element);
        }

which uses the name "files_joomla" to find out that we are updating Joomla itself.

Right, that's in the JInstaller library. Not the com_installer extension.

Should've clarified better. The Extension Manager (com_installer) MVC layer doesn't have any awareness of what you're installing. When the Extension Manager calls into the JInstaller library's install method, that's the first point the API gets any kind of awareness of what's being installed. So the MVC layer in itself can't detect the "if updating Joomla redirect to update component". And I would not encourage coding that condition into the files extension adapter either as it adds an inherent coupling between the installer library and com_installer extension, plus breaks the use of the extension manager as a fallback upgrade solution if it forces all core updates into the update extension. Anyone remember the 3.3.5 to 3.3.6 update? That's why, even though it's highly discouraged to NOT use the extension manager, a fallback needs to be available.

it's highly discouraged to NOT use

hmm, I would expect either

it's highly discouraged to use

or

it's highly encouraged to NOT use

at this point, or did I get it wrong?

@mbabker Absolutely agree with everything else you are saying!

Looking at the changes in the database driver there are routines to change the character set. Haven't found out if/where they are called. Sounds like those statements are meant to be ignored by schema management.

As far as NOT NULL DEFAULT "" is concerned: suggest to remove them completely, it doesn't make sense. Consider the usage a poor understanding of a RDBMS.

@richard67 My bad. Lots going through my mind. You extrapolated what I meant though, that's what matters.

@sovainfo The utf8 to utf8mb4 is parsed as a regular schema update so since the file follows the regular convention for that it gets picked up during a normal update routine. You're also right that right now the JSchemaChange* classes don't have awareness of what to do with these statements, in theory it should otherwise folks could conceivably always be told they have missing changes on their schema if they are not in a utf8mb4 supported environment.

Was referring to function alterDbCharacterSet in libraries/joomla/database/driver.php which suggest to convert when appropriate. Only can't find where it is used.

Considering the issues think that these statements don't belong to the sql files. They are not related to Joomla, they are related to MySQL.

Welcome to the Joomla update system and SQL management nightmare

Yes, that method specifically is never used right now. If the alter table commands were run in the PHP update script, not as SQL schema files, it would be used for the update process. The install app uses the getAlterDbCharacterSet() method to switch during installation. I don't remember why a SQL file was chosen over running it in PHP, and at this point I'm not interested enough to dig back through the history of issues/PRs to figure it out personally. I just know there's no strong argument to do it one way or the other with Joomla's current structure. Even Doctrine wouldn't bail us out of having this issue.

The point I was trying to make is that it shouldn't be in any update, It is ok for Joomla wanting to support this and even do all the work for you. It is not ok to connect it to an update of Joomla. Obviously you need to introduce this in an update, but that means the functionality should be introduced. It should help the administrator to use utf8mb4 in MySQL. Not force this on someone because the server supports it.

As soon as the requirements of Joomla are so high it requires a MySQL version having utf8mb4 you may standardize to it. Similar to the switch of MyISAM to Innodb.

I am still experimenting on what I can do to solve the problems but am not sure if I ever will succeed.

But I don't give up yet.

@infograf768 If you have some time, can you do me the favour and test following:

• In file 3.5.0-2015-07-01.sql, line 90 as in this PR, remove the " DEFAULT ''" from the statement for the #__banners table,

• replace the file in the update container if you used that

• check if still the statement for the #__menu table is the show stopper or if now the #__banners table is the one.

• Let me know the result pls.

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9170.}

2 problems to be solved:

1. The database schema manager cannot handle correctly indexes dropped and then later being added with the same name. It will always detect that such an index exists when coming to the check of the update sql file where it was dropped, before checking the one where it was added again.

2. When trying to work aroung that by making the database schema manager able to handle the drop and add in 1 single alter table statement, like i tried here in my PR, then it may happen that if for some reason the index to be dropped does not exist, the complete statement fails. This is standard behavior of mysql and happens when the sql is executed on update and also later when trying to fix that with the schema manager, and so results in a endless loop of having unsolved database problem not able to be fixed.

Because it would require a complete redesign of how the schema works for all kinds of databases so solve the 1st problem - we could make it queue statements for each "table.column" and "table.index" case in an associative array so later add index would superseed previous drop for the same index, and changes on the same columns in different sql files will be accumulated to at the end the extected result of all would be tested - this is much out of scope of my PR.

So I suggest an easier workaround now:

We drop all indexes "idx_alias" and add them with different name "idx_alias_100" (because limited to the 100) and also name them like this in the installation/sql/joomla.sql.

And have the statements in separate lines again.

The schema manager would not need any changes for this (except those I made to enable the alternative syntax "ADD UNIQUE KEY", "ADD UNIQUE INDEX" for "ADD UNIQUE" (without KEY or INDEX), and on a non-utf98mb4 database after the failed update (because the sql contains the invalid charset utf8mb4) the "Fix" would go through that step by step untill all is fixed, i.e. several times necessary to use "Fix" button but at the end all fine.

@mbabker @sovainfo and others: what do you think about that?

UPDATE: Hmm, I just see my index renaming would require to be done for postgresql and sqlsrv too.
Maybe nobody wants that.

P.S.: Of course there still has to be done something for the other statements for charset and and collation conversions.

In file 3.5.0-2015-07-01.sql, line 90 as in this PR, remove the " DEFAULT ''" from the statement for the #__banners table,

No change. same issue with menu.

[Unrelated: I wonder what is the use of the alias for banners...]

@infograf768 Hmm, can you try same test with categories instead of banners table?

I just see for the banners the alias is not really used and there is no index on that.

I would expect you to get problems with categories table instead of menu table when you changed that for the caegories.

Can you try that?

nope.

So I did a bit of playing around this morning on the train into work to try and deal with the first issue you mentioned Richard.

This is the query that is been run (and we simply here are checking if a result is returned as to if the index exists) SHOW INDEXES IN#__menuWHERE Key_name="idx_alias". If you actually look at the data that comes back from the query:

The top row is 3.4.8, the bottom row is 3.5.0. You can see that actually Sub_part contains the length of the key. This means that it's much simpler than anything you did with the "all in one line" query. You can have it as two parts. But we need to modify the mysql.php to do the query

SHOW INDEXES IN#__menuWHERE Key_name="idx_alias" ANDSub_part=100

where the 100 is parsed from the sql file. This makes it instantly more generic and easier to apply to other extensions. And solves the issue of index's being dropped and readded because it now recognizes that they are separate alias!

@wilsonge Ok, got you. Will try that.

@wilsonge But it will not help @infograf768 . He seems to have an issue without the DEFAULT '' for the menu table's alias column. And comments on other sql files suggest to me that it is a mysql issue and regardless of the discussions above we might need that. I would like to add this back in this PR at least for testing purpose, we can remove that later. Would you be ok with that?

Regarding the changes I did in mysql.php for the "ADD UNIQUE KEY" syntax: I can roll that back, too, and correct syntax in the 3.5.0-2015-07-01.sql, but then we have to make sure nobody uses that syntax in future.

Let me know what you prefer.

But it will not help @infograf768 . He seems to have an issue without the DEFAULT '' for the menu table's alias column

I am not alone with this issue. ;)

@infograf768 I know, but you are the most prominent example for me

@wilsonge Hmm, and how will your suggested change work on the unique "idx_client_id_parent_id_alias_language" over multiple columns of table "#__menu"? Your query will return a record for each of the columns I guess.

sorry, typo in post before, will correct via gihub comnment edit.

@wilsonge I just checked the multi-column index in myphpadmin and yes, it would require to loop over multiple records to build a query and also later to check the result.

@wilsonge Why not do a simple hack: We already have a hardcoded use of the name of this sql file somewhere. The schema manager runs this at the end of a fix action to do utf8mb4 conversion. So why not do a hack for this particular update file so ingnore the droop index or drop key statements because the indexes will be added in the same file later again?

Because JSchemaChange* isn't a library JUST to validate the core schema, its API is designed to allow extensions to use it too. So you need to be careful about adding these types of hacks.

@mbabker Then the only way I see out is to drop the indexes and use new names when adding them, e.g. idx_alias_j350, but this would require the names to be changed in the installation/sql/joomla.sql and update sql scripts for postgresql and sqlsrv. The suffix _j350 to the name would be - in opposite to a _100 - be independent from mysql and so not be confusing the other databases users and show it came with 3.5.0.

Can't say I'm a fan of field/index/table/whatever names using version numbers but honestly the way Joomla's structured all I can do anymore is ¯\_(ツ)_/¯ at that

Unfortunately I must say I fully agree with you, Michael.

And regarding the not null and default stuff: Whoever once worked with Oracle RDBMS or Microsoft SQL server will never dare to call MySQL an RDBMS.

This PR has received new commits.

CC: @810

This PR has received new commits.

CC: @810

Please ingore commit messages, I am still working on it so there is nothing to test yet.

Would rather say that whoever once worked with an RDBMS would never use NOT NULL DEFAULT "".

Worked on DB2,RDB,ALLBASE, Interbase, Starbase,Informix,Ingress,Sybase, SQL Server to just name a few. I have no problem to call MySQL an RDBMS.

It is not the RDBMS that is the problem here, it is the poor functionality provided by the schema manager. It is clear that the naming of Database->Fix doesn't meet the expectations site administrator have.
See no benefit of accepting multiple variations of the same statement. Would prefer the standardization on one syntax and maybe document it so people know.

Not convinced the schema manager fails on ALTER DROP, ADD ; It is already in use for PGSQL, maybe not tested properly! Considering that is one of my contributions I'll further investigate that.
Obviously the false positive is considered a known error of the schema manager!

@sovainfo The weak schema manager is another thing. Regarding MySQL and RDBMS I was referrring to what I had found out about how MySQL treats NULL values. As I said before, I agree not to use a default value like this, but it can be that MySQL forces us to do so.

The schema manager is inherently limited. It can't validate the full install schema; it's in a location that gets deleted after the install. Instead the JSchemaChange* classes are designed to use the series of update SQL files to validate that all updates (that it knows how to parse) have been applied. A proper schema management toolset would have awareness of what the full schema is supposed to look like.

@wilsonge Regarding the query for index properties we could maybe work based on following

SELECT
GROUP_CONCAT(column_name ORDER BY seq_in_index) AS columns,
GROUP_CONCAT(sub_part ORDER BY seq_in_index) AS subparts
FROM information_schema.statistics
WHERE table_name = 'j3ux0_menu'
AND index_name = 'idx_client_id_parent_id_alias_language';

It results in 1 record with columns = 'client_id,parent_id,alias,language' and and subparts = NULL on a 3.4.8 database, I have to check later for 3.5.0 where one of the subbparts should be 100.

The problem is that if I will really succeed in making the schema manager handle the update script for mysql, then the handling of scripts for other databases should be extended in the same way to have consistent behavior.

As much as I've harped on people for doing that (ensuring feature parity for all supported databases), unless more than 34 people^ show up looking for non-MySQL support I'm tempted to say screw it and don't bother.

^ Based on a raw query of the stats server database for stats received during the 3.5 beta period

We should but let's focus on getting the core CMS working first - none of this is required for non-mysql databases as they support utf8mb4 out the box :P

@richard67 What did you find out about MySQL treating NULL values?
Again, I would prefer a decent DEFAULT when possible and allow NULL values for non foreign keys.

@sovainfo See e.g. http://dev.mysql.com/doc/refman/5.7/en/data-type-defaults.html
It seems to depend on the SQL mode:

If strict mode is not enabled, MySQL sets the column to the implicit default value for the column data type.

And it seems without this stupid silly default value of emptry string for the session table's alias column we do not come forward, because @infograf768 abd @810 had tests where they changed this, and then it worked.
Do you know how we can check SQL mode?

@wilsonge what shall we do then? Try to make the schema manager clever enough for mysql (could really be complicated), or use new names for the indexes as I suggested somewhere above, e.g. idx_alias_j350?

Ummm I had considered falling back to a new alias name (although as michael said i'm sure we can come up with a more "intelligent" name than the joomla version...). I'm free all this evening. Once I get back from dinner I'll take some time to work through everything here :)

Bon apetit. Let me know if you have new suggestions and ideas.

This PR has received new commits.

CC: @810

OK So doing the query on the triple column index

This is my result in PHP MyAdmin. I think it's probably possible to build up a query BUT I agree it's starting to get pretty complicated. And it's probably worth sticking with the more "hacky" code that you've done, at least for 3.5

So have we worked out why JM's stuff is failing? I get that we have a failure when installing through extension manager. But I still haven't seen exactly why we are failing in other circumstances yet...

Well the problem with the query is that we do not get the necessary information from the sql statement in case of a single line "alter table ... drop index" statement, because this statement does not contain any more info than the table name and index name, so we cannot add more info to precisely find out if the index really had to be dropped or not. So it seems we have to stay with the 1 line statement for which i just added a correction in my latest commit. Otherwise if drop in a single line the schema manager will always complain that the index is still there when coming to that statement.
Dorp and add in 1 line has the disadvantage that if for some mistake the index is not there, the drop will fail, but the rest of the statement, too, so we always end at this point when schema manager has run. But this means our schema is corrupt and has to be repaired somehow anyway, so we would not need to handle that i think.
So we CAN add additinal info to the "create index" checks but for my solution with the 1 line statement it would not be required.
But what we should do and what i am just working on is to enhance the modify column check by additional attributes like "not null" or "default" or - and this is important - the "character set" and "collate" stuff. This could be added to the check query, and for the message output we append that to the type in the message, otherwise we would have to extend the message output by additional query types.

I'm not stressed about the scenario when if the drop fails the add fails. Whatever way you do things that should happen. In my case with the triple index you'd split things up into two queries. And you'd still have to rely on the index being dropped elsewhere. All you'd do is check that the index with those length constraints doesn't exist... (it sucks equally hard in other ways - but i think it's more technically correct).

Either way. Why is JM's stuff failing. Because he's not going through Extension Manager to install the update as I understand. So why is his stuff failing.

Secondly the installation through extension manager also doesn't make sense to me. I understand that the database driver is out of date. HOWEVER the index changes should still work without any issues as the schema manager (as I understand) would be at version 3.5.0....

HOWEVER the index changes should still work without any issues as the schema manager (as I understand) would be at version 3.5.0....

Nope. The JSchemaChange* classes aren't used in JInstaller to run extension updates. Those classes are only used to validate the schema is at the expected state. So if JInstaller and its adapters needed any changes to account for utf8mb4 scenarios you'd run into trouble too.

JInstaller doesn't need changes other than running the converstion from UTF8MB4 to UTF8. 04f09d5#diff-f1f11bb9d3a776c3980b1e188acd8b1dR954. So I'd completely understand that there would be errors when using extension manager to update if you don't have utf8mb4 support BUT the error that the index is too long shouldn't be the error because we should always have changed the index to be limited to 100 chars.....

JM can you give us your MySQL server and client versions please? Also what method of updating are you using? I'm assuming you are using the database fix button rather than upgrading files through extension manager?

This PR has received new commits.

CC: @810

I have added now the handling of the character set conversion statements to the db schema manager for mysql, because it was the easier task than what remains now:

The handling of additonal table attributes (like "NOT NULL", "DEFAULT" and "COLLATE" has to be added to the handling of "alter table .. modify .." statements of the db schema manager for mysql. This could be a bit more complicated because they may appear in different order.

If that all is done, the schema manager would be able to perform all the statements and so the additional call of the conversion script at the end of processing all updates can be removed.

What can be tested now with the last commit is that then "ALTER TABLE ... CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci" statements are counted as successfully checked database statement and not as it was before as skipped in the database manager's "Other Information" view.

I know that it is a bit a hack to add the collation to the table name for the messahes in the db schema manager list of problems, but a nicer output would require a new query type and new translatable message formatting.

You will see my hack when you test on a database not supporting utf8mb4 in the list or database problems after the update.

The rest mentioned above I will do tomorrow.

@wilsonge If you find some time, check in my PR the code deltas and let me know if I am on a good way.

.. or maybe I do that now .. is in the middle of the night here but I do not have to work tomorrow and not can stop now.

This PR has received new commits.

CC: @810

Fails:

Warning: Database is not up to date!

23 Database Problems Found.

Other Information

•Table 'gt8z9_banner_tracks' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_content_frontpage' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms0' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms1' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms2' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms3' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms4' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms5' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms6' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms7' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms8' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_terms9' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_termsa' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_termsb' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_termsc' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_termsd' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_termse' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_links_termsf' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_finder_taxonomy_map' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_modules_menu' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_ucm_base' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_update_sites_extensions' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)
•Table 'gt8z9_user_usergroup_map' (COLLATION 'utf8mb4_general_ci') does not exist. (From file 3.5.0-2015-07-01.sql.)

This will not be updated

@810 What means "This will not be updated"? Dioes it mean the database "Fix" button does not fix them? Here it does. If in your case not, try to log out from backend and then log in again after a while and try again the "Fix" button. It should run the statements (and if no utf8mb4 support replace utf8mb4 by utf8 in charset and collation so they should run).

From J 3.4.8
Replaced by GitHub files (staging).

I'm seeing 81 issues. When click Fix. its going down to 24. Then i click again on Fix. its going to 23 issues. And that is the final number when clicking "Fix"

@810 Hmm your screenshot shows lots of non core tables related to community builder or something like that?

I cannot really see where it stopped.

Can you show me a screenshot of what the database schema manager's "Other Information" tab shows, so I can see which are the 23 remaining issues?

Yes, this is my cb test site.
gt8z9_banner_clients and others . Updated the pics

@810 I cannot replicate this, neither using Joomla! Updater or the extention installer when using my update zip container (links see above).

The update zip container is my patch here applied to latest staging.

I get all database problems fixed here, on noth kinds of databases with and without utf8mb4 support.

Did you really use a clean 3.4.8 for the update test?

@810 Hmm your screenshots shows that for the 1st 4 conversion statements for tables #__assets, #__associations, #__banners and #__banner_clients have worked, and then the statement for the #__banner_tracks table failed. I have no explanation for that.

Was there some error reported after the update and before using the "Fix" button?

There was no error before that, i just tried with admin tools to change the collation but it doesn't changed it either. So now i testing with clean install.

1) Clean J3.4.8
2) Update to J3.5.0 Beta 2
3) Installed patcher
4) Apply your patch
5) Fix database: 31 issues
6) Result: 23 issues left

Same issue's

@810 My patch should not be applied with the patch tester on a 3.5.0. beta 2. You either have to use my zip container or my custom update URL as described in the test description at the top, or test as descibed by George (also mentioned in my test descriptions).

ok, im testing that package:
im using the utf8_general_ci database col.

Nope, still the same 23 errors

@810 Did you use the extension installer to install my zip file? Or the Joomla! Update component using the URL to my XML file as custom URL? Here works both like a charm.
If possible I'd like to know if there are problems with both or only with the extenstion installer for you, so if it is not rtoo much work for you let me know which metod you used for the previous test and if you can and want, try the other method and let me know the result.
If it is too much work I can understand.
So or so thanks for testing up to now.

i tried extension installer.

ok, tried both:
with update component, i get also this error:
0 Failed to start the session: already started by PHP.

Do you have skype, then we can chat.. mine jelle810

The "failed to start the session" is normal. If you click it, you should see the backend login, and if you login and go to the database view and click the "Fix" button, we know more.

Can you quickly check that?

I have skype but is almost 3am here so I will go sleep soon.

If necessary we can skype tomorrow.

Maybe we have more tests then and so know more.

It also might be an issue with PHP versions, that what I do with the "SHOW TABLE STATUS" is not compatible with all we have to support, I am not 100% sure about that.

Yes the same error appears 23 issue left.

Ok is good

Hmm, OK, sounds not good. Either I have a stupid error in my code changes, or it is not compatible, or we have earth rays or bad karma or saturn is in conjunction with the mooon or whatever.

I'll try to find out what I can after sleep, in some 8 hours or so.

We will see if someone else tests, too, and if their tests show the same or not.

Thanks and good night.

Ok i updated:
wamp 3.0.0 -> 3.0.3
mysql 5.7.9 -> 5.7.11
Apache 2.4.17 -> 2.4.18

And now its working, so maybe there was a bug.

•168 database changes were checked successfully.
•144 database changes did not alter table structure and were skipped.

Confirmed, when go back to mysql 5.7.9. it will not complete the sql runs. When updating, it runs smoothly.

@wilsonge

JM can you give us your MySQL server and client versions please? Also what method of updating are you using? I'm assuming you are using the database fix button rather than upgrading files through extension manager?

You are correct. I overwrite my test site (already as staging) with last staging and then click the Fix button.

Here:
Database Version 5.5.25
Database Collation utf8_general_ci
Database Connection Collation utf8mb4_general_ci

This PR has received new commits.

CC: @810

@810 Could you fall back to the database version where it did not work and test again? Maybe the problems was related to some columns to be converted had data which failed to be converted because columns were not enlarged before. I have changed order of processing in the SQL.

@infograf768 Could you also test again with the setup which did not work in previous tests? Changes I mentioned for @810 could help in your case, too, and I have added a statement for the alias column of the menu table to be sure there is norecord existing having a null value for some mistake and thus not allow an alter table modify for the column without a default. Maybe it works now?

Installed a 3.4.8 site.
Overwrote with staging and Fix database: same error.
Patched with your PR: same error.

This PR has received new commits.

CC: @810

JM what's the client version (i.e. the php<->mysql version)

@wilsonge

@infograf768 Well, the joomla.sql for new installation also defines the alias column of the menu table with not null constraint and no default value. So if this missing default value was the reason for the problem you have and @aschkenasy had with his test number 2, then you should also run in trouble when trying to make a new install with latest staging.

Do you? Or does it work for you?

I assume it works for you because otherwise I guess you would have opened an issue for that.

But then I am confused why the same thing works with a create table not not with an alter table with the same column attributes and the same mysql server and client versions and so on.

I can install a new staging fine.

@infograf768 That's what I expected, I only asked to be 100% sure.

But then this is a mystery to me.

Let's wait if @810 's tests which failed before work now.

For the thing with the alias column we might have to use the workaround to define a default value = empty string, I see no other chance right now.

Maybe then another, later alter table to remove this default will work?

If I only had a way to set up my system so it behaves as your does, then I could test myself better.

Question: If you enter following statements in myphpadmin, what are the results?

SELECT @@GLOBAL.sql_mode;
SELECT @@SESSION.sql_mode;

Btw until the commits this morning richard I couldn't do the database fix anymore. I narrowed it down to the index check in the schema file breaking again (won't have time to check today's commit's until this evening though :( - although will see if i can squeeze some time for it into my lunch break).

Let's try plan B - can you try and set up a separate PR (so this thing is still around for a best case scenario) where we change the names of the index's and see what differences that makes please?

@wilsonge

... this evening ... lunch break

Which time zone are you in? I am CET = UTC+1 here.

I don't see it in your GitHub profile, or I am blind

I'm in the UK - so GMT - so 10.45 atm :)

@wilsonge UK is good - I was scared it could be USA (blush).

I would try to fix and finalise this one here today (according to my time zone) and then if not ok do the plan B later tonight or tomorrow. Which scheme for the index name do you suggest? "idx_alias_100" (length-related)? Or "idx_alias_j35" (Joomla! version-related)? Or "ix_alias" (without the d so still neutral name, no version in it), or "index_alias" or so?

Regarding new PR: Meanwhile this one here is a bit messed up with my million commits - if this works and will be merged I will have the top score among the committers, and nobody will understand when reading the many partly silly and experimental commits, so I maybe should at the end make a new clean one for this here, too.

Just as an FYI - this is where the code was failing earlier if you do want to debug:

If we are dropping and recreating the index - then the else doesn't trigger and so we are simply checking if the index exists (which it does) so we don't drop and re-create the index.

RE: This PR - it's ok - before committing i can squash it all into 1 commit if it works :)

Finally I'd definitely don't want the Joomla version in it. I think i prefer ix_alias (or related variants). But we need to make sure we put a comment in the mysql files (especially install) explaining why we are deviating from the convention so in the future people don't try and change it back by accident to "standardize" :P

@wilsonge

Regarding naming scheme for plan B:

Maybe "idx_alias100", so in the multicolumn unique "idx_client_id_parent_id_alias100_language", i.e. no confusing underscore?

Regarding failed code:

It is not necessary to check for the dropped index, but could be nice and safer because then it would allow drop and add in separate lines in general. But as long as we have it in 1 line it is sufficient to check the details of the finally added index. If those not fit to the database, then the drop also has failed and has to be re-executed by the schema manager, and he will do so because is same line as the add.

But what is necessary is to extend the schema manager's check of the index by checking the columns list and the relevant lengths (subparts) of the columns. Then the schema manager would see that this "... drop ..., add ..." statement has not been executed yet.

And I have to extend the handling of column modifications (MODIFY) and renaming plus modifications (CHANGE) by including additional coumn attributes (NOT NULL, DEFAULT, CHARACTER SET, COLLATE) into the check query so the schema manager will run these statement, too, if necessary, and then at the end no final additioanl hardcoded call of the conversion script like is is appened now to the hole procedure is necessary anymore.

That convention is fine by me. It sucks. But it's the best of a even worse set of options right now....

Regarding the code: it meant for me when running the database fixer the changes to the index's never got executed and I now completely fail doing the database checks..... :(

@wilsonge

Regarding the code: it meant for me when running the indexer the changes to the index's never got executed and I now completely fail doing the database checks..... :(

What means indexer never got executed? You mean rebuild the mysql index in phpmyadmin? or mean the schema manager?

and I now completely fail doing the database checks

"Now" means with my code as it is now?

Sorry "now" means at the end of your commits last night. So not with the two extra commits this morning

on 5.2.9 its still failing

This PR has received new commits.

CC: @810

now 25 issues left

@wilsonge Do you have any idea why in the staging's 3.5.0-2015-07-01.sql (and so also here, I did not change that) the UNIQUE KEY idx_link_old was canged? It was already limited to the first 100 before, so there is no change to touch it.

I am just preparing the plan B branch and will remove the modification of that unique key from 3.5.0-2015-07-01.sql, and not rename this index, if I do not get a veto from you.

@wilsonge I have meanwhile created a branch for plan B (new index names):

https://github.com/richard67/joomla-cms/tree/correct-utf8mb4-plan-b

See richard67@staging...richard67:correct-utf8mb4-plan-b for a quick view on my changes.

But I have not made it a PR yet, I would like to have you a look on it first.

Also that new plan contains an extension of the schema manager to handle the character set and collation conversion statements for the tables, and I have removed the hard-coded run of the conversion again at the end of the database schema handling (this has hidden much here when I tested).

In principle the changed code should be able to handle everything, but now something strange happens here:

When I update a copy of my life site (which once was created with 2.5 and then with each available update updated and now is 3.4.8) to an update container based on my new branch, I get 30 database problems reported, of which the first one is following:

Table 'j3ux0_menu' does not have index 'idx_client_id_parent_id_alias_language'. (From file 2.5.0-2011-12-24.sql.)

The others then are the statements from the utf8mb4 update SQL.

Problem is that the statement which dropped the index mentioned above and the other one created it with new name has already been run, so for sure this index does not exist anymore with this name, and so the database schema manager always stops at this point mentioned above and never comes forward.

Maybe it is a problem from long time ago that this index never was there?

I could fix that and see how it goes on, but:

Why for heaven's sake does the schema manager checks and complains about an update script for 2.5 when updating a 3.4.8?

Do you have any idea?

P.S.: The index was there before the update, I just checked. So if this old SQL would not be checked all was fine.

@wilsonge What I found out with the test of the other branch I mentioned in previous 2 comments is that it seems to be related to data. When testing with a copy of my life site on a utf8mb4 capable db, I get index too long problems with the redirect links, of which I have many and long ones.

On the other hands others had problems where I had not.

The problems I see now have been hidden before by this final hardcoded call of the conversion script again, which I removed on the other branch.

So it seems as soon as we have long strings and then run the "alter table ... convert to character set" statements, the one or other of them crashes, depending on data.

If then the schema manager wants to repair that, he only executes those statements which failed, but not the other ones, and all comes out of order.

Even if we drop the indexes before the table conversions (like now) but then add them back later not like now before but then after the table conversion (which could solve this data-dependent key too long problem) and so have everything in order when the script runs for the 1st time, things go out of order as soon as 1 single statement fails when the schema managers then tries to repair tha.

I see no other chance than decoupling it somehow from the update files, mabe not doing it with sql script but with php code.

And as long as Joomla does not abandon support for "utf8 without mb4", the update scripts should use old charsets and collations or not try to change them at all, so that on systems not updated yet the update will work well, and then after update the utf8mb4 conversion could be done as soon as updated from a 3.4.x to a 3.5.y or so.

Maybe have something like the EOL plugin for 2.5, which shows info and a button to perform the conversion manually as ologn as not done yet, and then disappears when done?

Finish work in an hour, will have a second take on what you've written then as it's not sinking in atm :)

@wilsonge What I have found out is that if we wanna make the db schema manager to correctly detect things, then so or so, with old or new index names, it needs to include details of columns (not null, default, collation) and indexes (included columns and their subpart lengths) into the check query, i.e. if we add something details obtained from the update query text, and if we drop something (index or column). The problem is where to get details from when we drop some index of remove some column, I think actual database when building the ckeck query might not be sufficient, it would be better to go chronologically through the update sql files and when we drop something which was added in a preceeding sxcript, the details of what had to be dropped have to come from this preceeding script. But we do not have info about the starting point, i.e. how some index was once before it then should have been droppeb by a sopne aged 2.5-2011-xx-yy.sql script. We have to elaborate on that.

But the other problem why for some testers other table conversion statements fail than for some other testers, i.e. it depends on data. E.g. here I have in one test database redirect links filled with long URLs, but not much other data, and so this fails for me, while others have the problem with menu table or categories or whatever: As soon as the alter table convert to bla bla statement runs and any text column filled closed to its max size with text (regardless if in an index or not) will be converted whith this statement, it leads to an sql error (not sure if always or only if column is in some index).

So we have to enlarge ALL text columns, not only those which are utf8mb4_bin or which go into some index, to avoid data loss, but at least all columns which go into an index and the index length would be exceeded, and there it seems we have no catched all in the sql.

I remember with this old meanwhile closed PR #8472 we had found many things, but later it also had many changes which were not required, so I have to look into it and try so sort it out.

@infograf768 @810 @aschkenasy When you tested to update the 3.4.8, which data did the 3.4.8 include? Was it a copy of a life site with your special data, or was it a clean new install with some example data chosen at installation? If the latter: Which kind of example data? Testing? It seems it is data dependend for whom which table conversion fails.

@infograf768 @810 @aschkenasy P.S. to question in previous post: And did you do the additional step and added some languages at installation?

@wilsonge Ah, and again regarding my else clause you mentioned: I still think we should handle the combined drop and add of the indexes in this way that we do not check for the drop, because the index should exist after this statement. If we would check for the drop we could not use the details for the index from the actual data because this is the one we for sure not will have dropped (because it's the one later created). We have no chance to find out which were the details of something dropped without having initial database status and goint through all the history of the update files again, but we don't have this initial status.

The only reason why this fails now is that we only check if the index has been added, but not for the details from the update query (columns and their subpart lengths). As soon as we have this, it will work, and the schema manager will try to execute again the combined statemet (drop and add).

So I still think it is enough to check for the add part statement of such a combined statement, and so never go into this else. This else is only for those drop statements where no add iss appended, and there we still can live with the old situation that if we once dropped an index we should never add it back with the same name in anther statement. But for the combined statments it works as soon as we check for the details, too.

Last time I tested, was with 3.4.8 and Sample data, no other languages.

@infograf768 Which sample data? There are several options like "... Blog" or so from "None" to "Testing", I do not remember the names of the options in detail now.

Testing

Ok, thanks @infograf768

This PR has received new commits.

CC: @810

@810 Nothing to test, just rebased my branch

I meanwhile know that all the time we did things in the wrong order.

Here comes now how it has to be done, see also https://mathiasbynens.be/notes/mysql-utf8mb4:

Step 1: We have to limit indexes to the first 100 characters first, e.g.

ALTER TABLE #__categories DROP KEY idx_alias, ADD KEY idx_alias (alias(100));

Step 2:Then we have to enlarge the columns, e.g.

ALTER TABLE #__categories MODIFY alias varchar(400) NOT NULL DEFAULT '';

These first 2 steps we might have to do for more columns and indexes as we currently handle in the sql.

Step 3: Then we can convert the tables (i.e. all text or varcjar columns), e.g.

ALTER TABLE #__categories CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;

Step 4: Then we have to make the binary collated columns collate to utf8mb4_bin, because now after the previous step they have utf8mb4_general_ci.

Because of the limitations of mysql in syntax when modifying columns, we have to specify all column details again, and also use "CHARACTER SET utf8mb4 COLLATE utf8mb4_bin" and not "COLLATE utf8mb4_bin" alone, even if after the previous step the charset is already utf8mb4, because MySQL synrax requires this.

E.g.:

ALTER TABLE #__categories MODIFY alias varchar(400) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '';

And so the order of processing was not right in the sql, we enlarged the columns at the end for utf8mb4_bin collated columns, but we have to enlarge before converting the table.

@wilsonge @aschkenasy and who else is in the mood for it and has time: Please read and check and let me know if I am right or not.

I will prepare things here (what I mentioned here plus required changes in schema manager to handle things correctly) and first test on my own, this may take 1 or 2 days, then I might make a new PR or update this one.

Please be patient.

@infograf768 @810 @aschkenasy P.S. to question in previous post: And did you do the additional step and added some languages at installation?

No just default clean install, with sample data

Did not add languages at install time

This PR has received new commits.

CC: @810

@810 and all others: Please don't test yet. There is still something to do for index detail handling in schema manager, as I done now for column detail handling, but I am on a good way and think later tonight or tomorrow I will have this PR ready.

@wilsonge Please be patient, things are on a good way here.

This PR has received new commits.

CC: @810

This PR has received new commits.

CC: @810

@810 and all others: Please still don't test yet. Stuff is almost ready, but a bit has to be done still.

I just checked in to have it saved ober night.

@wilsonge Please be still patient, I'm almost done.

This PR has received new commits.

CC: @810

This is a lot more like what I was thinking of before! I like it!

This line https://github.com/joomla/joomla-cms/pull/9170/files#diff-789bdb944292f051222ae2d3e906033bR538 needs to be

$tmpStr = substr($tmpStr, $firstBracketPos + 1, strlen($tmpStr));

Otherwise you grab the index info not the column info :)

So once that line is fixed you end up with something like:

`SELECT s.`index_name`, s.`col_list` FROM (SELECT `table_name`, `index_name`, GROUP_CONCAT(LOWER(IFNULL(CONCAT(`column_name`, '(' ,`sub_part`, ')'), `column_name`)) ORDER BY seq_in_index) AS `col_list` FROM information_schema.statistics WHERE `table_name` = 'jos_menu' AND `index_name` = 'idx_alias' GROUP BY `table_name`,`index_name`) AS s`

I've ommited the where clause at the end and added the select of the col_list to try and get a feel for why the query is failing. I get a result from this

So it looks like somehow the column name is repeating itself into infinity :/

@wilsonge Right .. thanks ... for the substr correction.

Regarding the query: Here it works in myphpadmin, see following screenshots.

• Single column, same example as you chose:

• The multiple column index of the menu table as before the change, i.e. no sub_part length:

• The same as before but with a sub_part lenght:

This PR has received new commits.

CC: @810

@wilsonge Ah maybe you have several databases with the same table prefix, and I am missing to specify the database name in the information schema query, because this is a global thing?

@wilsonge Yeah, that's it, column "table_schema" contains the database name! You know from scatch how to get it in Joomla API?

JFactory::getConfig()->get('db');

That would explain a lot - currently getting

which should be the same as your second query...

This PR has received new commits.

CC: @810

@810 and others: Please not test yet. Work still on progress.

@wilsonge Now the schema manager detects what I expect, but I run into the same problem as JM now, and adding a silly default value does not help.

After I have added the columns and index details check, the schema manager did not report me anything from this old 2.5 update file I have mentioned some 3 meters above here in the thread.

So reported db problems look correct now, but still a bit stuff to do.

This looks a lot better in terms of just testing individual queries. Will test as a whole first thing tomorrow :)

@810 Thanks.

@wilsonge I had not noticed before that you already had made a PR to my branch for the substr correction when I corrected it myself. Sorry, did not wanna ingore it, but GitHub notified me too slowly.

This PR has received new commits.

CC: @810

So, done for now, need sleep.

What is missing still is checking default values and comments on column modifications, and in principle I should check the same details as for column modifications also for newly added columns.

This will not have any impact on actual update files processing, because there is nothing ambigious in history regarding columns which would require those checks now, so in principle things could be tested now without these missing addons.

But in future somone may want to modify column comments in an update sql and then 1 version later wants to modify it again. Same with defauls values. That's why I think I should do that tomorrow.

When all this here will be finished and merged, such things like modifying indexes and columns several times within the update sql files will be possible.

The danger in it is that my changes are not a fully implemented SQL parser in MySQL dialect, and so even if I tried to catch most common possibilities for statements, there might be still valid alternative syntax which is not covered yet.

This means poeple may think they can enter any valid SQL in the update SQL and append more things in alter table statements, because it so often works, but then it may not work for some reason, and without debugging in code nobody will know the reason why something failed.

What is also not so nice is that the output of details is just appended somehow to the name in the message is not nice (but anything else would require additonal query types and parameters and maybe language strings).

And finally I am sure my code could be made more elegant, using preg_replace and impolde/explode instead of what I do with ifs and elses and maybe also the loops, but I don't care for that now.

So a bit to do to finish this now, and then if all works much potential for future PR(s) to make it nicer then.

Hmm, when I test with @wilsonge 's method, I get the same error as JM.

But when I use Joomla! Updater I get following 2 database problems reported:

1. Table 'j3ux0_menu' does not have index 'idx_client_id_parent_id_alias_language'(client_id,parent_id,alias,language). (From file 2.5.0-2011-12-24.sql.)
2. Table 'j3ux0_user_profiles' does not have column 'profile_value' with type 'TEXT' null=FALSE. (From file 3.3.4-2014-08-03.sql.)

The first one is clear: We have now added the same index with different details, and so the check of the old update SQL tells that the old version is not found anymore.

And because we do not know what details and index once had when it was dropped, we might have to use the changes from this PR here, plus the renaming of the indexes from plan B.

But in general I still do not understand why when upgrading from a 3.4.8, scripts from far before this version are checked, for a version from which there is no direct update path in between. Or can we directly go from a 2.5. to a 3.4? As far as I remember not, there was an intermediate 3.x.y which had to be made.

If this was not the case, at least the 1st of the 2 issues would maybe not occur.

@wilsonge I meanwhile have updated my branch for the plan B with new index names, but it solves not the problems I have with the old update sql (1. issue in previous comment).

When the schema manager tries to re-run this statement, it fails in case of utf8mb4 support because the index will be created by this old sql without the lenght limit, and so it fails, regardless if we use old or new names.

If you wanna test that, too, you can find this plan b branch still here https://github.com/richard67/joomla-cms/tree/correct-utf8mb4-plan-b

This PR has received new commits.

CC: @810

OK. We solve issue one by commenting that "upgrade" out. Honestly there shouldn't be anyone running 2.5.0 anymore (if they are their site is probably hacked), and they should go to 2.5.28 before they go to 3.x anyhow in which case they will get the index. I'm not saying it's ideal - but we're taking far too long on this so let's make some tough decisions :)

But in general I still do not understand why when upgrading from a 3.4.8, scripts from far before this version are checked, for a version from which there is no direct update path in between. Or can we directly go from a 2.5. to a 3.4? As far as I remember not, there was an intermediate 3.x.y which had to be made.

I know you can go from 2.5.28 to 3.4.x (haven't done 3.4.8 explicitly)

A really clean solution would be not to process the statements list after it has been built, but loop through them and save their array index in an associative list, indexed by "tablename.indexname" or "tablename.columnname", together with which kind of statement was the last one (add or drop).

Then the schema manager would go through the statements again from old to young , as it is now, but for each "drop" it could check if there was an "add" before for the same tablename.indexname and if so, mark this as to be skipped, and otherwise, if it is a "drop", check if there was an "add" before and if so, mark this one as to be skipped. This check would not require

Maybe it would be more efficient to do this check from young to old statements and files and not from old to young, but so or so we should not loop through the array for each of these checks (would require for each statement to go back the hole list) but use associative lists (array indexed by this tablename.indexname thing, referencing the index in the big statement array).

Another thing is that for non-uft8mb4 databases the update will always fail and have to be fixed with the schema manager because we have the update scripts in the utf8mb4 versions, but the old 3.4.x or whatever to be upgraded does not have the code to check for it and if necessary replace "utf8mb4" by "utf8".

If we would agree to keep all update sql in the version for old utf8 (no mb4), then this would not be a problem. The only thing is that mainainers and contributors would have to know about it, that if they add something new, it will be with utf8mb4 in the installation joomla.sql but with utf8 in the update sql.

Ths would have to be keps as long as old non-utf8mb4 versions of mysql are supported, or until a new mayor version which requries migration and not directly updates.

The replacement would have to be a bit more clever, we would have to replace "utf8_bin" and "utf8_general_ci" as words, and not just utf8 by utf8mb4, in order not to replace "utf8mb4" with "utf8utf8mb4" or so.

Problem with all of this and comment above is, we do not have the time to engineer and test that, it would be such big changes that it would require good testing by many people, so 3.5 would come in 2017.

Finally what makes me crazy is that when I apply the conversion sql myself in phpmyadmin, everthing runs without a problem, and at the end when checking indexes and columns and collations all is applied, but when using Joomla! updater or Extension Installer (or the method you suggested for testing here), and in those 2 cases the problems are different.

Maybe it really depends somhow on the SQL_MODE? I have seen it being set somewhere in sql or php, have to see if I can find that again.

This PR has received new commits.

CC: @810

@810 and others: Was just a rebase, nothing to test again yet.

Here my test results from updating a copy of my life site from 3.4.8 to this PR for both kinds of databases with and without utf8mb4, using Joomla! Updater and my custom download URL.

UTF8MB4

PHP error session already started -> login again -> 2 database problems found

Table 'j3ux0_menu' does not have index 'idx_client_id_parent_id_alias_language' (client_id,parent_id,alias,language). (From file 2.5.0-2011-12-24.sql.)
Table 'j3ux0_user_profiles' does not have column 'profile_value' with type 'TEXT' null=FALSE. (From file 3.3.4-2014-08-03.sql.)

Fix -> SQL error 1061

Duplicate key name 'idx_client_id_parent_id_alias_language' SQL=ALTER TABLE `#__menu` ADD UNIQUE `idx_client_id_parent_id_alias_language` ( `client_id` , `parent_id` , `alias` , `language` );

No UTF8MB4

SQL error unknown charset utf8mb4 -> 11 database problems found

Database schema version (3.4.0-2015-02-26) does not match CMS version (3.5.0-2015-11-05).

Table 'j3ux0_menu' does not have index 'idx_client_id_parent_id_alias_language' (client_id,parent_id,alias,language). (From file 2.5.0-2011-12-24.sql.)
Table 'j3ux0_banners' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_categories' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_contact_details' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_content' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_menu' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_newsfeeds' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_tags' does not have column 'alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_ucm_content' does not have column 'core_alias' with type 'varchar(400)' COLLATION='utf8_bin' null=FALSE. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_contentitem_tag_map' should not have index 'idx_tag'. (From file 3.5.0-2015-10-26.sql.)
Table 'j3ux0_contentitem_tag_map' should not have index 'idx_type'. (From file 3.5.0-2015-10-26.sql.)

Fix -> Same as above for UTF8MB4 database: SQL error 1061

Duplicate key name 'idx_client_id_parent_id_alias_language' SQL=ALTER TABLE `#__menu` ADD UNIQUE `idx_client_id_parent_id_alias_language` ( `client_id` , `parent_id` , `alias` , `language` );

Conclusion

The result is the same, so after my corrections for the schema manager things on both databases seem to be consistent and predictable now and not different and strange as before.

A simple solution for the problem with the outdated update sqls could be to make the schema manager stop checking the files as soon as for one file there was an error.

When fixing the problem then the schema manager could mark that file as fixed either in its internal array or, if it shall be safe, in an own new database table maybe.

With the nex run of the check the successfully fixed file will not be checked anymore, but then those coming afterwards (and not being fixed before because stop after 1st file with problems).

Now maybe another, later file will report errors, and so we fix those, and if ok mark the file as not being processed.

And so on and so on.

This will result in database problems to be fixed step by step, i.e. not at once, but will make sure that we not process or check things out of order, e.g. by running checks for 2.5 files again.

@wilsonge What do you think?

I think your rebase has gone wrong :P

Ummm I need to sit down and have a think for an hour before I reply quickly to that. My gut is that I agree with you. But it's not simple....

@wilsonge I also have do something else for an hour or 2 before I can check my rebase. If all went wrong I should maybe make a new PR and close this one.

@wilsonge Regarding the rebase: Diff looks good here with Beyond Compare (only can recommend this tool). Only commit history seems to be crazy, and the "composer dependency changed" flag which this pr has now also confuses me a bit. Do you know what tthis means?

It means you changed a composer file. If you look at the diff - there are 46 files changed according to github and one of them is richard67@1782a31 which changed the composer dependencies

Oh, maybe that came from one of the recent PRs to staging?

What do you suggest? Close this PR here and make a new one? Is maybe the easiest things.

OK I have merged the SQL changes - because this should at least fix the Joomla Update component c9a6081 and I might do another beta whilst we work on the schema stuff

@wilsonge
FYI: c9a6081 has not solved the issue with menu here.

Yeah that will fix the Joomla update component as I said in the commit. It won't fix the database fixer tool or installing via extension manager :/

@wilsonge Well, as it is after this merge, I works because there is still this additional call of the file for utf8mb4 converion at the end, with ignoring exceptions so the script will be processed statement by statement without caring if error or not, which I had removed in my PRs.

So the status is we have the same SQL file once somewhere in the sequence of sql files and then again run at the end of the sequence. This hack from Roland will work for now, but as soon as some other sql changes something which also is conerned by the update sql, it will not fit anymore, i.e. it would need a different file for the end of the sequence than used within the secuence.

Example: Someone changed again or removes one of the inxedes modified in the update sql and adds this to a file being run in the sequence after the update sql. If he/she then changes the update script, too, to reflect his/her other change, then it will succeed at the end but fail in the middle of the sequence (and this we will see), or if he/she does not change it, it will succeed in the middle but then fail at the end (which we not will see because of the ignored exceptions).

So if we wanna keep some additional script run at the end, we need a separate script for this, which may look same as the 2015-07-01 script now but different in future, and this script should not be in the sequence of update files and so be in a different folder. We (Roland together with us testers) tried something similar in this old meanwhile abandoned PR #8472.

But I still think we should remove that additional hardcoded run at the end.

@wilsonge What could be done instead at the end of all instead of running a script could be to 1. Loop through all core tables and save info about which tables not have utf8mb4_general_ci collation yet and which columns of these tables have utf8_bin collation, then 2. convert tables found in step 1 to character set utf8mb4 and collation utf8mb4_general_ci and then all the columns found in step one to character set utf8mb4 and collation utf8mb4_bin. The problem could be to know what are core tables and what are tables belonging to 3rd party extensions.

Honestly I can see us going back to #8472 - this schema stuff just doesn't want to work. I had another long chat with @nikosdion yesterday and it turns out that INFORMATION_SCHEMA access is restricted on a lot of shared hosts. Swapping that part of the query to get the same data from SHOW INDEXES also doesn't work (mysql just errors). So it would seemingly leave us in a place where we can't detect whether index's have changed or not, so the best place to go is to revert back to having things in a separate SQL file like we had in #8472 and hack some sort of feature detection into the com_installer part of the database fixer.... :(

Unless you have any better ideas of course?

We can hardcode a list of core tables in the worst case. There's seemingly no "nice" solution to this. So revert back to hack where essential...... :(

@wils and one final thing: Whenm woring on the shema manager I found that the existing version (before my changes) had an error: When checking for "ALTER TABLE .. CHANGE", is uses the old and not the new column name to build the check query if the column exists (see that by comparing with the preceeding check of "ALTER TABLE ... MODIFY", the same index for the column name is used (= wrong), but for the column type then index in "CHECK" statement is 1 after the index in "MODIY" (= correct).

The reason why nobody ever noticed is that we not have any update sql where "CHANGE" is used to rename a column when modifying it, we only have statements where it is used to modify the column and so a MODIFY statements would have been sufficient.

I did not make a PR for that because 1. I planned to modify the functions anyway with my PR here or maybe another one for plan B, so the error would have been corrected, and 2. it would require hacking an sql file for test and so on, while it would be easier to check it just by code review.

So I will make a comment now on your commit to show you what I mean, maybe you can do that without a new PR and testing overhead just by review.

@wilsonge Ah, I cannot cmment in your commit because it does not include the mysql.php

I mean lines 121 and 123 in file libraries/cms/schema/changeitem/mysql.php, where "$wordArray[4]" should be "$wordArray[4]".

Can you check that? Or shall I make a PR? I'd like to avoid the PR because it would require some test instructions to be written.

Do the PR - but i'll merge it on review if I think it's all good!(so no test instructions needed ;) )

@wilsonge New PR for what I mentioned in my previous 2 comments is #9220

@wilsonge Regarding what you found out: Well, it was also my concern with limited access to the information_schema tables.

What do you think about following:

1. We place a Readme.txt (or .md) in the folder of the update sql scripts for mysql, telling that any new stuff has to be added to the update scripts with sizes suitable for utf8mb4 but in case of binary collated columns still with "character set utf8 collate utf8_bin", and that in case of such binary collated columns then the conversion statement has to be added to the one and only separate conversion script.

2. We use this new one separate conersion script outside the update scripts folder for making the conversion at the end, as it is done now with the 3.5.0-2015-07-01.sql file.

3. This handling has to be kept until Joomla! one day drops support for old mysql version not supporting utf8mb4, e.g. with 4.0

4. For new installations everything in joomla.sql will be made for utf8mb4 like now, because on new installation the statement modification (check if utf8mb4 is supported and if not, replace 'utf8mb4' by 'utf8' in the statements) will work. So new cinary collated columns will be with utf8mb4 in the joomla.sql but with utf8 in the update script. Also this should be explained in the Readme mentioned above.

The big advantage of this solution will be that Joomla! Updates will work on non-utf8mb4 systems with extension installer or Joomla! Update component without any problems, what would not have been the case with any of the things we tried here and with plan b or before with the old PRs.

What do you think? Can we do it like this? If yes, I will make a new PR.

When JDatabaseDriverMysql[i] is confronted with utf8mb4 data but the server
doesn't report utf8mb4 compatibility the query is automatically downgraded
to utf8 character set / utf8_general_ci collation.
See convertUtf8mb4QueryToUtf8 in JDatabaseDriver. It is used in
JInstaller::parseSQLFiles and JSchemaChangeitem::fix.

Therefore all Joomla! SQL scripts must use the utf8mb4 character set and
the utf8mb4_unicode_ci or utf8mb4_general_ci collation for CREATE TABLE and
ALTER TABLE statements. You *may *have to update JSchemaChangeitem to put
all read SQL statements through convertUtf8mb4QueryToUtf8 to avoid it
complaining about the character set / collation on servers which lack
utf8mb4 support.

@nikosdion This is true if it will be the updated database driver running after updating a pre-3.5 to a 3.5 or later. Is this granted, i.e. the new driver is used for sure after the update has been unpacked? If so, you are right regarding this, but still it makes sense to separate the conversion from the update scripts into a separate script, and when someone adds a new binary collated table column or a new table, he/she also has to add a conversion statement to the conversion script. I am just preparing a PR for this separation (without the readme yet).

@nikosdion @wilsonge See new PR #9221 - please review, I will test here meanwhile and the update test instructions.

Clarification: I was talking explicitly about what to do with Joomla! SQL
files from 3.5.1 onwards. Before 3.5.0 is released we do need a separate
SQL file with the utf8 to utf8mb4 conversions. I only wrote my previous
comments because you were (apparently?) saying that all schema altering SQL
commands must be written in regular files with utf8 collation and to the
special file with utf8mb4 collation. I am just saying that in the future
this won't be necessary.

Regarding your question, the only scenario where a non-UTF8MB4-capable
database driver is used is when the Joomla! update is installed as an
extension. IMHO this upgrade path should be deprecated and dropped in
3.6.0. Furthermore, if you end up in this situation and the upgrade to
utf8mb4 fails we can *still *fix that using the script.php in com_admin
which does get called by the restore.php script of com_joomlaupdate. And,
to be honest, this upgrade path is why we are trying to get the friggin'
JSchemaUpdate::fix to work with the utf8 to utf8mb4 upgrades: if someone
installs Joomla! as an extension of itself and the schema update fails they
will need to use Extensions, Manage, Database, Fix to complete the upgrade.

So, recap:
NOW - We need to separate utf8 and utf8mb4 and get the bloody JSchemaUpdate
working.
IN THE FUTURE - All schema update queries (create or later table) will be
written in utf8mb4 only, in the main SQL files.

@nikosdion Done, see my new PR #9221 mentioned in my previous comment just before yours.

But still in future it can happen someone updates a 3.4.8 to e.g. 3.5.1. Will this work with future update scripts using utf8mb4? If not, we should then force them to go via 3.5.0.

I commented on #9221.

Regarding your question, that kind of failure is not a problem. We cannot
create a new table anyway due to b/c constraints of version 3.x and we
cannot change existing columns for the same reason. This leaves us with the
case of adding columns. I'd say that if this is the case we should let the
schema Fix button handle the failure instead of asking core developers to
put SQL statements in THREE (3) places (installation, com_admin and the
special utf8mb4 SQL file). I mean, we already have one place too many for
schema changes, let's not completely screw up core developers to cater for
a very small percentage of our users when there's already an alternative
solution for them (schema fixer).

Nicholas K. Dionysopoulos
Director, Akeeba Ltd
https://www.AkeebaBackup.com

On Fri, Feb 26, 2016 at 2:55 PM, Richard Fath notifications@github.com
wrote:

@nikosdion https://github.com/nikosdion Done, see my new PR #9221
#9221 mentioned in my previous
comment just before yours.

But still in future it can happen someone updates a 3.4.8 to e.g. 3.5.1.
Will this work with future update scripts using utf8mb4? If not, we should
then force them to go via 3.5.0.

—
Reply to this email directly or view it on GitHub
#9170 (comment).

@nikosdion Thanks for your comment. We can continue discussion at my new PR #9221.

@wilsonge I will delete now my branch "correct-utf8mb4-plan-b" where I provided the plan B with new index names for testing. It did not have a PR yet, and I messed it up the same way as this PR here with my rebase, and if necessary I have the changes saved here so can restore them in a new PR if necessary.

Can only hope that PLT doesn't follow @nikosdion advice to deprecate installing an update as an extension. If something needs deprecation it is the schema manager. It is plainly wrong, doesn't deliver on its promise. Suggest to deprecate it right now (J3.5). No use trying to fix that poorly implemented functionality. It is far too limited in the technical implementation it is right now.

Can only hope that PLT doesn't follow @nikosdion advice to deprecate installing an update as an extension.

It needs to happen sooner than later. Running an entire core update and being reliant on the application to update itself with regards to filesystem (and API) changes causes more issues than it solves, especially when you introduce new features into the API. Just off the top of my head, we've had to force the legacy JApplication class near the top of the autoloader stack to deal with its file location changing between 2.5 and 3.x and the old version being loaded otherwise, plugins can't rely on $this->app or $this->db being populated correctly, and now this mess. Granted, the database changes should be using the API, but the filesystem updates for the core application should have NEVER been given to the extension install library to handle. That's a bad design decision dating back to 2010.

If something needs deprecation it is the schema manager.

Yes and no. It's not a schema manager, let's not degrade real schema managers by calling it one. It's a changeset tracker. That library at best is designed to ensure all schema change SQL queries are in the current database. What is really needed is a proper schema awareness and moving the location of the install SQL to a place that it is available after installation so that whatever API is in place is aware of what the full schema is supposed to look like, not just tracking whether changesets have been applied.

@sovainfo Cutting your head off is not a valid way to stop a headache, no
matter how effective it is! Both items need to be deprecated. Ironically,
only the install-Joomla!-as-extension problem -which you don't want us to
solve for whatever stupid reason- already has a solution included in
Joomla! for the last 3 years or so :p

I have already told George that the schema updater needs to be shelved
since two and a half years ago. I have even written my own solution which
I'm using in my own software. Put nicely, the schema updater and the whole
concept of one SQL file per release needs to be killed with fire. Everyone
agrees on that but a. nobody agrees on a "proper" solution and b. backwards
compatibility in the 3.x range means that we can't apply a proper solution
until 4.0 (which basically means that the current 4.0 development effort
must be renamed to 5.0 to not let the solution stagnate forever).

Installing Joomla! as an extension to itself is also completely moronic and
dates back to the ultimate updater cop out in Joomla! 1.6.0. It was a
massively bad idea! The CMS is NOT an extension to itself. The update to
the CMS needs to take place outside the main CMS execution loop. We
accepted this circa 2.5.4 when com_joomlaupdate was finally included in the
CMS. The only reason we have the old update method is that a. nobody cared
to remove it and b. we never told people that yes, you CAN upload the
update package manually and com_joomlaupdate will find it and use it. There
is no compelling technical reason to allow installing Joomla! as an
extension to itself while several reasons exist to finally stop allowing
that outright idiotic practice.

If you wonder why installing Joomla! as an extension to itself is stupid:
you are replacing classes on disk BUT you are using the old code of said
classes to install the update. This means that any bug introduced in
version X does not allow you to install version X+1 unless you can travel
back in time and fix version X, in which case you wouldn't need to release
version X+1. This has gotten us many times in the past and is currently
being a massive PITA for the utf8mb4 upgrade, since the old code does not
know about utf8mb4 and the update has to run through that old code, leading
to a chicken and egg situation. Also what Michael said. So the only
reasonable course of action is to kill that stupid update path because it
simply does not work correctly!

@mbabker Not that long ago I recall you saying the Extension installer is a fallback many rely on when issues with Joomla! Update !. Many still prefer it that way.

Obviously, with the improvements of Joomla! Update and the ability to do the same as the Extension installer, nothing stops you from removing the duplicate functionality. It only requires administrators to get used to a different way of working. Not something they are well known for. Especially if they are not sold on it.

No matter how you call it, it simply doesn't deliver!. And calling it Database->Fix is the worst thing. Although it is technically accurate, it creates expectations it can't deliver. It is not a problem to find a situation where this works. The problem is that there are too many scenario's ( real live situations ) where it only makes things worse!. Instead of pointing out what the problem is, it hides the problem!

@mbabker Not that long ago I recall you saying the Extension installer is a fallback

It is, and should continue to work as such until a time that support is fully removed. As long as we treat Joomla as an extension of itself, using the Extension Manager should generally work. The issue is that if an update relies on updated API in a new version, this will fail catastrophically, and there really isn't anything that can be done about it given how JInstaller runs through an extension's update (extensions can update just fine in this way, core updates absolutely need the filesystem updates (extracting the new files and deleting the old ones) and everything else (schema updates mainly) split into two different processes). I'm all for dropping this support, but not in a "well we can't solve the utf8mb4 update issues in the Extension Manager so from now on we no longer support this" way that just comes up out of nowhere.

issues with Joomla! Update

Those need to be addressed. Ironically the code in the JInstaller library could still be used in the update component to actually download packages (which is right now done with the code Akeeba was using when this was contributed). Streamlining that fixes any package download related issues for both the Extension Manager and the update component, and anyone with their own update code that uses those functions. Can't help that folks would rather use an update mechanism that more often than not fails when introducing new features, they NEED to be trained to use the more robust system or go back to 1.5 style FTP updates.

No matter how you call it, it simply doesn't deliver!. And calling it Database->Fix is the worst thing.

Oh please don't get me started on this. I read a Reddit thread about a month ago where someone was having filesystem related issues and someone seriously told them to click that fix button as a solution. Between the naming of that button and the lackluster abilities of the library it's connected to, it needs a lot of attention.

@nikosdion Agree with deprecating both Updating through Extension->Install and Database->Fix.
Provided Joomla! Update and its updating as a separate extension has proven to work!
Also, believe the equivalent of Extension->Install can be achieved with a CUSTOM configuration of J! Update. There is no need to have so many different ways to do the same thing. It only needs one way that works!

Don't know why you come up with the cutting your head off to resolve your headache, nobody is suggesting to migrate to drupal!
Don't know what makes you say I don't want issues to be resolved, I do. That doesn't mean you can remove everything that is not perfect.

@mbabker Dropping Database->Fix comes up because it doesn't meet the requirements. It is a solution for problems we don't have anymore (manual update). Actually, it is not a solution, it is hiding the issues instead of solving. As mentioned it creates more problems than it solves. Consider it hurting Joomla more than benefiting.

Issues with Joomla! Update are not all software related, obviously those need to be addressed. Most of the time they are infrastructure related.
Not all infrastructure issues can be dealt with in a timely manner. Which is why it is nice to have alternatives available.

The biggest issue with Joomla! Update (and extension updates) is that due
to Thomas' stubborness we ship an incomplete cacert.pem which does NOT
allow downloads from Amazon S3 on about 10% of hosts out there. Joomla!'s
update files are of course stored in Amazon S3, hence the problem
downloading. Same goes for hosts who stupidly block all kinds of downloads
from external locations either by disabling URL fopen wrappers (instead of
URL includes), disabling cURL (proving they know nothing about web server
security) or, worse, firewall their servers in a way that disallows any
kind of downloads. All these boil down to "com_joomlaupdate doesn't work"
when in fact the problem is "my server sucks balls". However, as I said, it
is perfectly possible to download the update ZIP file manually and put it
in your server's tmp directory and com_joomlaupdate will find it. This
needs to be documented and all of the "com_joomlaupdate problems"
(actually: server problems!) are solved.

So the question is not whether we should kill the problematic method of
installing Joomla! as an extension of itself but why the heck haven't we
done so. I will say this again: com_joomlaupdate is THE way to install an
update. The download issues CAN be worked around. I very explicitly gave
that option foreseeing the problems. The major problem I have is that half
of the download problems I am not allowed to fix and half of them you don't
know but you are able to work around anyway.

Also I am not calling for the removal of a dangerous and broken update
method just because I can. I am doing that because it's dangerous and
broken. Unless you invent time travel you CANNOT realistically count on
installing Joomla! as an extension to itself to work reliably (unless we
stupidly assume that the JInstaller and JDatabase packages are not buggy,
will never be buggy and will never, ever, not in a million years require to
change, ever). Experience has proven that once we make anything more than
cosmetic changes we introduce a change that makes this update method FAIL
CATASTROPHICALLY, WITHOUT THE POSSIBILITY TO FIX IT IN ANY WAY WITHOUT TIME
TRAVEL. Michael already gave you an example from the past and I gave you a
current example too. If you know of a way to time travel and fix that code
before installing an update be my guest. I would rather stay in the side of
realism and propose to remove the broken by design update method. The
irony is that Mark Dexter had accepted the need to do that when I
contributed com_joomlaupdate all those years ago but then he quit and the
PLTs from that point onwards lacked the balls and technical acumen to make
this change happen.

So, there is ONE way that works. And I've proposed another one: extract all
files on top of old version; run a .php script either from the command line
OR from the browser which triggers script.php in com_admin and performs all
post-update actions. This would be the perfect fallback and, best of all,
it's already possible to do today (we've had the technology to do it
since 1.7 actually). So there, I've got you all the solutions.

Amazon updated their cert so that is no longer a problem
On 26 Feb 2016 3:39 pm, "Nicholas K. Dionysopoulos" <
notifications@github.com> wrote:

The biggest issue with Joomla! Update (and extension updates) is that due
to Thomas' stubborness we ship an incomplete cacert.pem which does NOT
allow downloads from Amazon S3 on about 10% of hosts out there. Joomla!'s
update files are of course stored in Amazon S3, hence the problem
downloading. Same goes for hosts who stupidly block all kinds of downloads
from external locations either by disabling URL fopen wrappers (instead of
URL includes), disabling cURL (proving they know nothing about web server
security) or, worse, firewall their servers in a way that disallows any
kind of downloads. All these boil down to "com_joomlaupdate doesn't work"
when in fact the problem is "my server sucks balls". However, as I said, it
is perfectly possible to download the update ZIP file manually and put it
in your server's tmp directory and com_joomlaupdate will find it. This
needs to be documented and all of the "com_joomlaupdate problems"
(actually: server problems!) are solved.

So the question is not whether we should kill the problematic method of
installing Joomla! as an extension of itself but why the heck haven't we
done so. I will say this again: com_joomlaupdate is THE way to install an
update. The download issues CAN be worked around. I very explicitly gave
that option foreseeing the problems. The major problem I have is that half
of the download problems I am not allowed to fix and half of them you don't
know but you are able to work around anyway.

Also I am not calling for the removal of a dangerous and broken update
method just because I can. I am doing that because it's dangerous and
broken. Unless you invent time travel you CANNOT realistically count on
installing Joomla! as an extension to itself to work reliably (unless we
stupidly assume that the JInstaller and JDatabase packages are not buggy,
will never be buggy and will never, ever, not in a million years require to
change, ever). Experience has proven that once we make anything more than
cosmetic changes we introduce a change that makes this update method FAIL
CATASTROPHICALLY, WITHOUT THE POSSIBILITY TO FIX IT IN ANY WAY WITHOUT TIME
TRAVEL. Michael already gave you an example from the past and I gave you a
current example too. If you know of a way to time travel and fix that code
before installing an update be my guest. I would rather stay in the side of
realism and propose to remove the broken by design update method. The
irony is that Mark Dexter had accepted the need to do that when I
contributed com_joomlaupdate all those years ago but then he quit and the
PLTs from that point onwards lacked the balls and technical acumen to make
this change happen.

So, there is ONE way that works. And I've proposed another one: extract all
files on top of old version; run a .php script either from the command line
OR from the browser which triggers script.php in com_admin and performs all
post-update actions. This would be the perfect fallback and, best of all,
it's already possible to do today (we've had the technology to do it
since 1.7 actually). So there, I've got you all the solutions.

—
Reply to this email directly or view it on GitHub
#9170 (comment).

It'd be nice to finally solve #8186 but I'm not holding my breath. I think we've hijacked this thread enough though.

All we need to do to do that is merge #8645 - however as @nikosdion was also warning me that JHttp had a issues on crappy hosts i've been very reluctant to merge it! Honestly once that is merged we can upgrade everything to https.....

Agree with all above that it is a waste of time to try to make Extension->Database(->Fix) do something it was not designed to do.
Suggest to concentrate on fixing real issues and document the known limitations!
The important thing is to be able to update (using Joomla! Update).

My new PR #9221 will be ready soon, so things will be fixed.

Trying to fix schema manager was not a waste of time for me because I learned which way we cannot really go and how my new PR has to solve it.

Closed in favour of #9221

But it also resulted in #9220 which should be reverted!

No, 9220 is correcting the error of current staging as it was before this PR here, so it is OK and should not be reverted.

Ok, I'll prove you are wrong in 9220.

Alter table syntax with change instead of modify allows to specify and old and a new column name, see http://dev.mysql.com/doc/refman/5.7/en/alter-table.html for mysql 5.7 but is true also for older versions.

If no renaming, both names are the same, but so or so 2 column names have to be given then using change, and so the coumn name is not in wordArray[4] but in wordArray[5], check my link and the code please!