Probably fixed by #9510

No, #9510 does not fix that I think.

OK, you're probably right reading that again. The query it's trying to execute probably comes from the 2.5.0-2011-12-24.sql schema diff (assumed since it's missing the length on the alias and that's the only other schema diff dealing with that index). So this one's probably got bigger issues to deal with.

@richard67 so you have to limit alias filed of that index to 75 or something?

Well the index should be there, and schema manager is not clever enough to check which columns with which lengths in the index, it only checks the name.

At the moment I have no idea what could have gone wrong here.

It only can be that some of the sql statements during conversion has failed on update, means when sript.php was run to perform the utf8mb4 conversion, so that the index was dropped but not re-created, and because of the exception caused by this, the conversion status was not set. But in this case the page should have shown at the top an sql error message.

The schema manager would then see the index is missing and think this comes from the old schema update script, and try to run it again on the already converted table, without the lengths limits for the alias column.

@andrepereiradasilva No, limit to max 191, or 100 as we did in core.

yes right forgot!

@wilsonge @mbabker @andrepereiradasilva Maybe we should change error handling of the utf8(mb4) conversion as follows:

1. Run the conversion at the end of an update (script.php) or a database fix (schema manager) only if no open db problems left at the end of the fix, i.e. new schema version is as expected, so we do not run conversion on a not up to date db schema.
2. For the 2nd script where currently exceptions are caught so that errors are shown in message field at the top, but processing of statements is continued when an error has occurred, we should exit the loop, i.e. stop processing of further statements when an error has happened with a statement.
3. Optinally we also could check after script 1 has run, where we only drop indexes without error reporting (for the case of a missing index caused by not correct old db), that after the script has run all indexes to be dropped really not exist anymore.

This would make sure we not run into trouble when running conversion on (partly) not updated db schema and not have inconsistent db after something goes wrong at conversion, e.g. caused by some of those magic mysql session default settings changing from mysql subversion to subversion as it seems.

Let me know if you like this idea, then I could make a PR.

I think could be very likely that the changes I mentioned above would help to avoid such scenarios as seen in this issue here.

I don't mind doing this in database fixer.

But script.php shouldn't care -we can't tell whether the sql for the current update has run properly or not - so there's no way of being able to tell whats happened as far as I know. And we have database fixer to deal with those edge cases.

@richard67 I will comment only based on my scarce knowledge of this, so if i say something that doesn't make sense please say so.

1. Run the conversion at the end of an update (script.php) or a database fix (schema manager) only if no open db problems left at the end of the fix, i.e. new schema version is as expected, so we do not run conversion on a not up to date db schema.

IMHO makes sense.
Only up to date db schemas should run the utf8 -> utf8mb4 conversion to avoid errors.

1. For the 2nd script where currently exceptions are caught so that errors are shown in message field at the top, but processing of statements is continued when an error has occurred, we should exit the loop, i.e. stop processing of further statements when an error has happened with a statement.

So, for what i understand, the utf8 -> utf8mb4 conversion should stop if any sql error exist.
If that's the case, what should an end user do when that happens? Go the database fixer, like they do now? If so why add this?

1. Optinally we also could check after script 1 has run, where we only drop indexes without error reporting (for the case of a missing index caused by not correct old db), that after the script has run all indexes to be dropped really not exist anymore.

Ok, for what i understand, so were there could be an half conversion.
The same question applies, if that's the case, what should an end user do when that happens? Go the database fixer, like they do now? If so why add this?

@lal12 If you need to fix the problem so you can continue with your site, open following sql script in an editor:

2.5.0-2011-12-24.sql

and add "-- " to the beginning of following line:

ALTER TABLE #__menu ADD UNIQUE idx_client_id_parent_id_alias_language ( client_id , parent_id , alias , language );

so it looks like this:

-- ALTER TABLE #__menu ADD UNIQUE idx_client_id_parent_id_alias_language ( client_id , parent_id , alias , language );

means comment it out.

Then go to "Extensions -> Manage -> Database".

Now use the Fix button.

Can you check if this procedure helps as a manual fix for the problem, and let us know the result?

And do you have a backup and a database dump of the status before you tried the update to 3.5.0, i.e. when it was 3.4.8?

If so, let us also know, and keep those, so you can help us later with testing if you want and if we have found a correction to solve the problem on updating.

Thanks in advance.

@andrepereiradasilva Well, of course if changed error handling ends with an error, the problem should be shown (together with the reason, i.e. the sql error message) at the top, and the database should be shown as not up to date.

But you are right, it would end more likely as it does now with the situation that the Fix button cannot solve a problem and so the problem remains forever until you manually patch one of the old update scripts which caused the problem. And when we would change the conversion error handling, people also might more likely have to hack those scripts, too.

So maybe it was not such a good idea.

That's why I asked you, bevause 2 brains thinking together about something have in total more than double performance than 1 brain only, because the 2 brains are different and so not only sum up their capabilities but also wider the range.

You know I am not that guru Joomla! developer that I just csn drop in a final solution which works, so I sometimes like to discuss things.

And thanks that you are willing to do so and help with your opinion.

I still think that you're first point makes a lot of sense.

Run the conversion at the end of an update (script.php) or a database fix (schema manager) only if no open db problems left at the end of the fix, i.e. new schema version is as expected, so we do not run conversion on a not up to date db schema.

Sure, that makes sense, because it does not create new problems with beign stuck with a db problem, like the other modifications would. As soon as the schema problem is solved, also the utf8mb4 conversion will be done.

On the other hand, just thought about it: Currently, when an error occurs with script 2, execution of statements continues, but the conversion status will be set to zero so also right now the conversion would be shown as open db problem until it once has run without any error (exception).

The difference between now and my proposed point 2 above would be that conversion would stop at that point with the error. Am not really sure if that would make sense or not, just wanted to point out that regarding having open db problem at the end, it would not make a difference.

@mbabker @wilsonge @andrepereiradasilva @aschkenasy

I think we can avoid such problems by changing the order of processing in the 2nd conversion script "administrator/components/com_admin/sql/others/mysql/utf8mb4-conversion-02.sql":

We could move the recreating of the indexes with their new limitations from before the character set conversions of the tables to after it.

This means we first drop in the 1st script the index, then in the 2nd script we resize some columns, then convert all tables, then modify the binary collated columns, then set the default charsets and collations of the tables, and then finally at the end re-create the indexes, i.e. not before conversion like now, so any "1071 Specified key was too long; max key length is 767 bytes" should not be possible anymore during the conversion.

The only remaining case when this could happen again would be if the database fixer again would apply an old schema update, e.g. "2.5.0-2011-12-24.sql" on a db where the affected table already has been converted to utf8mb4.

In such a case then nothing else helps than commenting out the statement which makes the problems in the old schema update sql, but this is nothing new, such fixes also were necessary in past if schema was not ok for some reasons.

A maybe positive or maybe unwanted side effect (Eli let me know your opinion about that if you find time) could be that mysql optimizes the table when after conversion an index is added on ac column?

If you think the proposed change in order of processing makes sense and could be helpful, or if you think the opposite, let me know please.

If all agree I will make a PR then.

it seems it should resolve the issues, but i leave this one for those that better understand more how db index and collation conversion works.

@richard67
Thanks for the advice. I tried it and it worked out for me, so just an error is shown in the Backend on the DB-Site:

"The Joomla core database table haven't been converted to UTF8 multibyte yet."

After clicking "fix" the core tables seem to be converted. Also after uncommenting again, the error is still gone.

I will try to retrieve the database backup. On the server runs a daily backup job, but it saves the all server files, so I am unsure, whether I am able to extract just the database.

@lal12 Thanks for the feedback. Let me know if you could find the backup. We might need help later for testing a future PR helping to avoid such problems, or helping to better fix them, or both.

New PR is #9549 . Is not ready for test yet but I will let you know soon when it is.

New PR now is #9590 . Please test.

Closing as we have a PR