AFAIK key and index is the same thing?

They are the same thing.
@wilsonge I thought that's what we said all along. It's unfortunately rebuilding the entire index, but we're forced doing it because of the length change.

Please note that due to cardinality change it might be necessary to run ANALYZE TABLE after an ALTER TABLE. Just FYI.

It was what we said all along. I know we need the 100 char key length - it's just working out how to work through the damn database fixer to get there... :(

https://github.com/joomla/joomla-cms/compare/fixsqlutf8mb4?expand=1 This was the kinda fix I was expecting to need to do (obviously this is only for one of the indexes for simplicity.) But it doesn't work.....

@wilsonge If I understand correctly your problem is that you think that you need to put the following SQL commands in the update .sql file:

ALTER TABLE `#__menu` DROP KEY `idx_alias`;
ALTER TABLE `#__menu` ADD KEY `idx_alias` (`alias`(100));

but Joomla Schema Manager doesn't support the keyword KEY, it only supports the keyword INDEX. However, KEY and INDEX are synonyms in MySQL:

KEY is normally a synonym for INDEX. The key attribute PRIMARY KEY can also be specified as just KEY when given in a column definition. This was implemented for compatibility with other database systems.

As long as you don't have a table's primary key on a VARCHAR column (thankfully we've got enough sanity left to not do that atrocity in Joomla! core) you don't have a problem! I mean that the above SQL statements could be written as:

ALTER TABLE `#__menu` DROP INDEX `idx_alias`;
ALTER TABLE `#__menu` ADD INDEX `idx_alias` (`alias`(100));

The SQL part of that works, I just tried. However, I can't try the whole update thing – I'm still sick and writing this takes up pretty much all my energy. Can you please try it and see how it works?

On it - thanks!

@wilsonge Let me know if I can help with tests later. @nikosdion I hope you will get better soon.

@wilsonge If you are busy with other stuff I also can help with coding, i.e. modify SQL as proposed by Nik and do a PR for that, either to the Joomla repo or your branch in your, whatever you prefer, just let me know.

I can work on it tonight - if you want to take a stab at it before then - please feel free! (just do it as a regular PR to the CMS)

@wilsonge I just start to work on it.

One problem is that not only 3.5.0-2015-07-01.sql contains statements "drop key" or "add key" not supported by the database fix but also the 2 old files 2.5.4-2012-03-19.sql and 3.0.0.sql contain such.

As far as I know we can change only those SQL files which have been added after 3.4.8, so I should not touch those old ones, right?

But then we should make sure that old 3.x.y < 3.4.8 have to update to 3.4.8 because they can go to 3.5, right? Or are these scripts so old that this is already covered by actual upgrade paths/restrictions?

@wilsonge And 2 other questions:

Is it really necessary to do the drop index and add index in 2 different alter table statements? From checking existing old SQL statements I would expect the database schema manager to be able to handle it in one line.

And should maybe both be done, fix the 3.5.0-2015-07-01.sql file so it can be applied with the current version of the database schema manager and fix the manager so in future people can use "drop key" and "add key" in future SQL files?

We can always go back and change those files - but let's focus on the 3.5.0 file for now :) Just do everything in the file we have.

Umm see how it goes. I think yes because if you do not then it won't recognise the add key (because the key already exists until you remove it on the previous line).

The schema manager is only used by us. Just use index's. If we are going to support keys separately again that can be done after we've shipped 3.5.0. Let's just get the minimum needed done to 3.4.0-2015-07-01.sql and the joomla.sql installation file (so we are in sync) to get the beta out :)

@wilsonge It seems that the database schema manager in general, i.e. neither for mysql nor for postgresql nor for sqlsrv supports having the "drop index" and "add index" within one "alter table" statement (or equivanlent), separated by comma.

We could try to make this possible but this would be a big impact and I am not sure if I can make that soon.

So for now we should split the statements into 2 and make the manager allow synonym KEY for INDEX for mysql, am just preparing both.

Or do you think we should go the perfect way and make the manager handle sub statements of alter table?

If not it would need a rule in our coding guide lines for SQL that drop and add of an index should not be done within 1 alter table statement.

What's failing in the schema mangager? The problem is that (as I put above just now) - that the add key statement won't get recognized because the OLD key/index already exists :/ (because the checks aren't smart enough to look against other statements made - they just check what's in the live database)

Hmm at least we could add a condition to the schema manager, would be a small change, see richard67@63c8a0c

Basically your problem is that Joomla's scheme checker only checks for the existence of an index by name and not its definition (columns and / or length). Isn't that what you mean, George?

Yes (from my quick overview on the train this morning anyhow)

@richard67 i did that already (see https://github.com/joomla/joomla-cms/compare/fixsqlutf8mb4?expand=1#diff-789bdb944292f051222ae2d3e906033bR72 - which was some doodlings i linked to above from the train this morning) - but it doesn't fix the problem here...

well one problem is that the database schema manager does not support having the drop and the add in the same alter table statement, as sub statements separated by comma. so they have to be in 2 lines. so the change you mentioned above is ok but not sufficient.

Yes but having them in two separate lines fails too because the schema checker checks for key name (brackets or not - https://github.com/joomla/joomla-cms/blob/0ca4214c04880b7e15b8cfaa9edf111da453bdae/libraries/cms/schema/changeitem/mysql.php#L76 ). This means that it thinks the key already exists. So as far as I can tell there is no way except to have them in one line (unless you have a better idea).

I don't understand. Exactly why does the schema checker fail in the all in one lines? (I know it doesn't - but this was one of the things I hadn't been able to work out in the time I had available)

@wilsonge Can you check staging...richard67:correct-sql-index-changes if you are on a desktop? I am preparing tests, and if it works, I make a PR of that.

I mean that's pretty much what I thought should work here https://github.com/joomla/joomla-cms/compare/fixsqlutf8mb4?expand=1 (as i said above i tried separate lines and "all in 1" lines - neither with success) - if it works tho awesome - I guess I just screwed up somewhere.

The easiest way I found to test was commit your changes to a branch. Then checkout 3.4.8, delete your database and reinstall a "clean" 3.4.8. Then check back out to the branch you made your changes on. Try running the database fix tool and see what happens.

@wilsonge Your link does not show any differences for me, it shows a form to create a PR instead.
And maybe you were on the right way and just forgot the change for some tables, it is not only the menu table to be done, it is also categories, tags and ucm_content.

@wilsonge I justed tested my stuff, it fails, too. I'll let you know if I have something new.

Hmm, it seems that when the database schema managers checks for consistency, it finds the drop key (or index) in 1 line and so thinks there should not be such index anymore in the database, and it ignores that in the next line another alter table statement adds the key (or index) again, so at the end of the Joomla update from 3.4.8 to my PR I end with following database inconsistencies:

Table 'j3ux0_categories' should not have index 'idx_alias'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_menu' should not have index 'idx_alias'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_menu' should not have index 'idx_client_id_parent_id_alias_language'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_menu' does not have index 'KEY'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_redirect_links' should not have index 'idx_link_old'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_redirect_links' does not have index 'KEY'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_tags' should not have index 'idx_alias'. (From file 3.5.0-2015-07-01.sql.)
Table 'j3ux0_ucm_content' should not have index 'idx_alias'. (From file 3.5.0-2015-07-01.sql.)

Any idea?

The SQL related to the text above was:

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

ALTER TABLE `#__menu` DROP KEY `idx_alias`;
ALTER TABLE `#__menu` ADD KEY `idx_alias` (`alias`(100));

ALTER TABLE `#__menu` DROP KEY `idx_client_id_parent_id_alias_language`;
ALTER TABLE `#__menu` ADD UNIQUE KEY `idx_client_id_parent_id_alias_language` (`client_id`,`parent_id`,`alias`(100),`language`);

ALTER TABLE `#__redirect_links` DROP KEY `idx_link_old`;
ALTER TABLE `#__redirect_links` ADD UNIQUE KEY `idx_link_old` (`old_url`(100));

ALTER TABLE `#__tags` DROP KEY `idx_alias`;
ALTER TABLE `#__tags` ADD KEY `idx_alias` (`alias`(100));

ALTER TABLE `#__ucm_content` DROP KEY `idx_alias`;
ALTER TABLE `#__ucm_content` ADD KEY `idx_alias` (`core_alias`(100));

So it seems the database schema manager always checks only 1 "alter table" statement for each table, and the first one wins.

OK, I just see 1 mistake in SQL is it should be "ADD UNIQUE ..." and not "ADD UNIQUE KEY ...".

No it's not the order it checks in. The issue is that you it does a check against the table to see if the key exists. In the case of the menu alias field for example, it checks to see if the key idx_alias exists. If it does then it doesn't try and execute that statement (which is what Nic says here #9156 (comment)). It doesn't check to see if the length of that key has changed (or even what the key is based on).....

Yes, but so or so I have found following errors on code in staging:
1. The one I mentioned just before, that the 3.4.0-2015-07-01.sql containts "ADD UNIQUE KEY", which is not recognized by the schema manager, and
2. that for some tables (e.g. "#__categories") there were statements missing to limit the index.
This has to be solved anyway and was not solved with what you tested.

And to solve the rest we either make the schema manager be anle to handle the drop index and add index in 1 line (and forbid having them in 2 lines within 1 sql file), or we split dropping and adding the indexes to 2 separate sql files (means for utf8mb4 conversion we than have to call those 2), and we forbid dropping an index and then creating it again within 1 sql file. Forbid would mean to add this to ourcoding guides.

As far as I could see this is a problem also for sqlsrv and postrgesql, that the schema manager cannot handle the check of dropping and creating of an index within the same sql file.

Hmm, even if I use separate files for drup and add index it does not work. So it seems we have to have the drop and add in the same line within 1 alter table statement and make the schema manager be able to handle that.

And we have to change 2 times "ADD UNIQUE KEY" to "ADD UNIQUE" in file 3.4.0-2015-07-01.sql.

Hmm, but "ADD UNIQUE KEY" or "ADD UNIQUE INDEX" seems to be valid syntax (beside "ADD UNIQUE"), so we have to make the schema manager handle that, too.

I started to work on another branch for that, the one with my tests i mentioned above does not work but you can check what I did so you know what will not work.

it doesn't even matter about if they are in the same sql file or not. it compares the changes to what is in the live database....

Yes, I found out meanwhile. I am currently preparing a PR, will let you know when I have a solution.

@wilsonge If you have time check this, I am preparing test meanwhile, and if this is ok I make the PR:
staging...richard67:correct-db-schema-manager-1

@wilsonge @nikosdion @aschkenasy

Please test PR #9170 (issue tracker)

@nikosdion Sorry, I forgot you are still sick.

Please, whoever has time, test PR #9170 .

I'm really sorry - I'm super busy with a real life deadline on Monday. I'll test it as soon as I have a free second!

Oh, good luck with your stuff then.
Any idea who else could be competent and willing to test this? If so, maybe you can ping them?

I know there was a recent commit trying to fix this, but in some unrelated testing I ran into the following after rebasing a branch to staging that had the new commit

The test took the rebased branch as an upgrade from J3.4.8 (so going from 3.4.8 to staging with another small unrelated change)
Database showed something like 23 fixible errors and this came after fixing

**An error has occurred.
 1071 Specified key was too long; max key length is 767 bytes SQL=ALTER TABLE `abc_menu` MODIFY `alias` varchar(400) NOT NULL COMMENT 'The SEF alias of the menu item.';
Call stack
#   Function    Location
1   JApplicationCms->execute()  C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\administrator\index.php:51
2   JApplicationAdministrator->doExecute()  C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\cms\application\cms.php:257
3   JApplicationAdministrator->dispatch()   C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\cms\application\administrator.php:152
4   JComponentHelper::renderComponent() C:\Bitnami\joomla-3.4.1-0\apps\joomla\htdocs\libraries\cms\application\administrator.php:98
5   JComponentHelper::executeComponent()    C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\cms\component\helper.php:380
6   require_once()  C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\cms\component\helper.php:405
7   JControllerLegacy->execute()    C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\administrator\components\com_installer\installer.php:19
8   InstallerControllerDatabase->fix()  C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\legacy\controller\legacy.php:728
9   InstallerModelDatabase->fix()   C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\administrator\components\com_installer\controllers\database.php:30
10  JSchemaChangeset->fix() C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\administrator\components\com_installer\models\database.php:60
11  JSchemaChangeitem->fix()    C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\cms\schema\changeset.php:129
12  JDatabaseDriverMysqli->execute()    C:\Bitnami\joomla-3.4.8-0\apps\joomla\htdocs\libraries\cms\schema\changeitem.php:246**

New PR for this 3.5.0-blocker is PR #9221 , please test. #9170 is obsolete and closed.

This issue here can be closed because the PR #9221 for it is well tested with success and has RTC label now.

Just as a reminder.

Thanks :) And thanks again for the huge number of hours you put into this Richard! It's really really appreciated :)

@wilsonge Well, I like to do such things from time to time. Unfortunately it is not predictable when I have time for it. It can happen that I do much now but then disappear for months before doing again something. So when I disappear one day, it does not mean I am angry or something or gave up with Joomla!, it is then just for example job-related. Just wanted to let you know that.

it's ok :) i think that goes for the majority on contributors - then there's just a couple of crazy suckers like me :P

@wilsonge @richard67 Both of which are like MasterCard moments. PRICELESS.
Thanks to everyone involved, Be that testers, commenters et al. but especially @richard67.

Well, at the moment I had a bit time and mood, and it was fun for me somehow ... a bit like when playing Lego when I was kid ...