I have tested this item ✅ successfully on caafc82

Unfortunately, this has no effect
-- it only stops the error from showing,

The schema checker

• neither reports nor executes cases that it does not recognize

This new statement with 2 index actions inside same statement, is non-recognized case for schema checker,
which tries to parse this query and it does not fall into any of its known cases

Effectively the existing INDEX is not dropped and the new index is not added

To see that they are ignored test with these non-recognized cases:

ALTER TABLE `#__languages` DROP INDEX `idx_imageAAAA`, ADD INDEX `idx_imageAAAA` (`image`);
ALTER TABLE `#__languages` DDDDROP INDEX `idx_image`, AAAADD INDEX `idx_image` (`image`);

The proper fix is to

• use the statement added by this PR,
• but also add to detection of multi-index (DROP / ADD) case to the schema checker

[EDIT]
i do not have time to do the above, but it should not be too difficult, and it seems that this work can not be avoided

aaa, no, my suggestion will not work either,
a more complex solution is needed

It can not be done using only SQL file to check proper index
It will have to be done in PHP code

you can not just drop and add the index the SAME index ?
how do you know if the existing index is correct or not correct ?

the SQL file includes no such check

aaa, solution is so simpler

1. drop old index named idx_image
2. add new index with a different name e.g. idx_image_nu

(_nu) is for _non_unique

no need to update schema checker PHP code at all

ALTER TABLE `#__languages` DROP INDEX `idx_image`;
ALTER TABLE `#__languages` ADD INDEX `idx_image_nu` (`image`);

[EDIT]
Do not forget to update the index name for new installations to be idx_image_nu

simple solution are always the best one

It doesn't necessary have to work with the DB fixer. That one doesn't work for other cases as well. It's only a bandaid to begin with.
As long as it works in an update from an official release (not nightlies, staging, betas, ...) and doesn't trigger a warning in the DB checker it is fine.

@Bakual

As long as it works in an update from an official release (not nightlies, staging, betas, ...) and doesn't trigger a warning in the DB checker it is fine.

I do not understand you, do you mean that this

UNIQUE KEY `idx_image` (`image`),

was added in J3.7.0-dev ? so no need to drop it

I checked and the above exists in the installation for J3.6.4 and before,
so since it does not come from a nightly build,
the DB-fixer is better to care about it too

Or you mean that since the upgrade SQL files are run by installer during upgrading only once,

• then we do not care about the DB fixer to recognize the SQL statement and you just let it be silently ignored ?

but the DB-fixer is exactly for this: to be able to do (almost all) the schema upgrading work, that was not done by the Joomla upgrading process for any reason, why leave out this case, if it is simple to add it ?

I mean, why would it be better to siliently ignore the SQL statement, and not making 2 queries so that upgrading can be done by DB-fixer too ?

then we do not care about the DB fixer to recognize the SQL statement and you just let it be silently ignored ?

That. The fixer for example also doesn't take care of insert/update/delete statements and we just ignore it. As long as the updater itself works, it is fine.

but the DB-fixer is exactly for this: to be able to do (almost all) the schema upgrading work, that was not done by the Joomla upgrading process for any reason, why leave out this case, if it is simple to add it ?

It's a bandaid for broken upgrades, yes. But as said it already doesn't work for all cases anyway.
If it is easy to do, then go for it. But if we end up with non-intuitive names for the index (like the image_nu), imho it isn't worth it.
A better fix would be to fix the DB fixer ?

That. The fixer for example also doesn't take care of insert/update/delete statements and we just ignore it. As long as the updater itself works, it is fine.

Yes right, DB-data related queries are not supported,
(it would be too complex / tricky / impossible to support them in the "DB fixer")

but DB-fixer is meant to only check for DB-schema changes
and (i think) up to now, it supports all DB-schema changing queries, right ?

and (i think) up to now, it supports all DB-schema changing queries, right ?

Apparently not, otherwise we wouldn't discuss this here :)

For it to work properly it requires a space before the comma. Now it doesn't recognize the name properly.
You might want to leave it like that to prevent it claiming it needs fixing every time.

Suggest to deprecate the schema checker due to its poor implementation. It simply doesn't deliver on the promise. The biggest problem being all those people running it expecting it to fix a failed update.

Instead of checking the database structure whether it is what it is supposed to be, it TRIES to detect whether structural changes have been applied. Apart from failing to properly recognize structural changes it is also implemented inconsistently among the databases.

Instead of checking the database structure whether it is what it is supposed to be, it TRIES to detect whether structural changes have been applied. Apart from failing to properly recognize structural changes it is also implemented inconsistently among the databases.

Feedback on the idea in #12578 would be beneficial. If I can ever find another 12 hours in the day or a clone of myself I'd like to see this fixed.

For it to work properly it requires a space before the comma. Now it doesn't recognize the name properly.
You might want to leave it like that to prevent it claiming it needs fixing every time.

I tested, and by adding space before comma,
it does NOT recognized 2 index changes in a single statement,

(i maybe wrong here, but a quick check at the code of the fixer does not reveal it to support comma for multiple index ADD/DROP inside same statement)

libraries/cms/schema/changeitem/mysql.php
libraries/cms/schema/changeitem/postgres.php
libraries/cms/schema/changeitem/sqlsrv.php

-- but even if you put the 2 statements into 1 and they do get recognized, it is not of big importance

The important thing is to use a new index name, so that the fixing will only run once,
that is regardless if you make the change with 1 statement or with 2 statements

Also the DB-fixer is a not a DATA/SCHEMA fixer !

• it is only a SCHEMA fixer not a DATA fixer !

About not supporting all SCHEMA fixes,

• so far in the SQL files, (please correct me here) only SCHEMA statements that are supported / recognized by the SCHEMA fixer are used in these files

Can you find a SCHEMA changing statement that is inside existing Joomla SQL upgrade files that is not supported by the fixer ? (maybe there is a case for non-MySQL)

Suggest to deprecate the schema checker due to its poor implementation

Ok improve and maybe replace, but not supporting all cases is not a "poor implementation"

• it is an incomplete implementation,

"poor" should judged on different criteria

Ok improve and maybe replace, but not supporting all cases is not a "poor implementation"

• it is an incomplete implementation,

"poor" should judged on different criteria

It's a poor implementation because it relies on change deltas to work and cannot work with a primary source (for core that'd be joomla.sql). That single fact alone is why I won't implement checking the database's schema state in my extensions.

It's a poor implementation because it relies on change deltas to work and cannot work with a primary source

Yes , i see, that is indeed a good explanation why it is poor

but checking the change deltas, is all that is needed in vast majority of the cases that a user will need to fix the DB schema

[EDIT]
of course someone has to write these upgrade SQL files, instead of just changing the primary source: joomla.sql, and that is extra work

In the majority of cases, yes. However, checking the schema's state should ultimately rely on a single authoritative source. We don't have every database changing transaction in our deltas (if we did, there would be an initial state where at least all of the tables are created in their initial state), so it cannot be relied on as an authoritative source.

Even in projects using Doctrine as a database layer, their API does not rely on (optional) change deltas (their migrations system) to ensure the validity of the schema, it uses an authoritative source (generally you're working with their ORM so the source isn't a single SQL file but rather the mapping data defined in your project which gets compiled to the correct SQL structure and diff'd against the active database).

Originally these files were introduced to apply both structural and data changes upon update. The arrival of ->Extension manager->Database(->Fix) imposed some restrictions on the structural statements.
Hopefully, the current testing process is that good to ensure there are no structural statements in those files that are ignored by the schema checker. Not that it matters much, the fact it ignores the data changes should have been reason enough to never allow this in the product.
So far haven't seen documentation about those imposed restrictions. Also not convinced these restrictions should be accepted. Consider the update process the owner of the data, not the poorly implemented schema checker that does a very poor job!

Consider that people are better of without it, advice people not to use it. It is very likely getting you in worse situation. No matter what it reports, it is not reliable!

@andrepereiradasilva

Regardless of when and how the Database Fixer will be deprecated,
such a thing will not happen in this small PR

Since the current code of this PR can not be handled by the DB Fixer (as explained above it is not recognized),
and since nobody suggested that the following code does not work (which i tested and explained why it works)

ALTER TABLE `#__languages` DROP INDEX `idx_image`;
ALTER TABLE `#__languages` ADD INDEX `idx_image_nu` (`image`);

do you think that you can update this PR ? (and also update the joomla.sql file to use the new name of the index)

yes i know that would work. since there is no alternative for now, i think we need to go that way.

repalced by #13252