Collection of fixes for the update script

[auge8472]

Currently there are a few new threads with error reports about the update script in the project forum. We can identify three main issues.

• [x] 1. A few users ran into a trap of an edge case. They had previously tested a development version of the 2.5-branch (2.4.99.x) and downgraded their forum to a stable version of the 2.4-branch again. Normally one would restore an database backup of the 2.4.x version that ran before and ignore the remaining tables of the 2.5-branch from the test. But this breaks the update script because we didn't take account of such a upgrade-downgrade game.
• [x] 2. Issues with the order of SQL statements for altering the table stucture. Changing the charset to a value that breaks the maximal index size before defining the charset espacially for an affected column and limiting its size to a hazard-free value is not a good idea. So one task is to check the order of the database queries.
• [x] 3. Defining a column as UNIQUE without checking the real values in the database table for doublettes breaks the update script and leaves the database in an intermediate state. Some changes were made, others not. Check for doublettes before trying to alter the definition of the column.

This PR should address these problems.

[auge8472]

About 1.:

I added a rigorous DROP TABLE IF EXISTS statement in front of the CREATE TABLE statement for the four new tables in 2.5. At this point (update from a version from 2.4.19.1 up to 2.4.24) none of these tables should exist but if they do (because of a previous test) they should be resetted to being completely new. As additional safety level I put a IF NOT EXISTS into the CREATE TABLE statement itself. This should be not necessary because of the previous DROP TABLE but it should also not break anything. Nevertheless I am open for a discussion of this step. So it is IMHO not a problem to delete a probably existing table with all its outdated content and to create it again.

To ensure the operation to be safe (having no TOCTOU problem) I enclosed the two queries (DROP TABLE and CREATE TABLE) into a transaction. That led to a MySQL syntax error, that needed to be solved with the use mysqli_multi_query instead of mysqli_query for querying the operation.

To keep the statement blocks readable I reformatted them and put the statements in variables instead of noting them inside the function calls.

$qCreate_Table = "TRANSACTION;
DROP TABLE IF EXISTS t1;
CREATE TABLE IF NOT EXISTS t1 (…);
COMMIT;";
if (!@mysqli_multi_query($connid, $qCreate_Table)) {…};

[auge8472]

Transactions does not work with tables of type MyISAM. Because of that we changed the table type of the existing tables to InnoDB which supports transactions. But this is at a later point of the update script during the update to 2.4.99.2 or 2.4.99.3. At the point, where I want to (re)-create the new tables (update to 2.4.99.0) I can not be sure about the table type of the previously created tables. Normally I would simply assume, that the previously created tables for the 2.5-branch are new enough to fall under "modern" default settings of a MySQL or MariaDB-server (default charset UTF-8, default table type InnoDB) because all servers I have access to are cofigured that way. But at the end I can not be sure about this.

So my question is, if we can reorder the database queries for changes of the database structure in the update procedure without breaking anything. IMHO it should be possible to reorder at least the changes of the several pre releases (2.4.99.x) without breaking anything. But I am not sure.

@loesler Can you please also take a look in the update script to find possible pitfalls? Thank you.

[loesler]

I can not be sure about the table type of the previously created tables.

I'm sorry, I don't really understand the problem of your post. Do you like to know the ENGINE of the tables? If so, you can select the used one via, e.g.,

SELECT TABLE_NAME,
       ENGINE
FROM   information_schema.TABLES
WHERE  TABLE_SCHEMA = '<YourDataBaseName>';

Having such a list, it is possible to check the table type (and to adapt the type if required).

[auge8472]

I'm sorry, I don't really understand the problem of your post.

I think about changing the position of the ALTER-statements for changing the table type to the start of the database operations, directly behind creating the db_settings.php. But writing this, I think, this is a bad idea and unnecessary. There is completely no TOCTOU problem at this point. I can delete a possibly existing table and create it afterwards again without any problem without a transaction (that would require a proper table type).

I will change the code again. Thank you for the input.

[loesler]

Looks comprehensible to me.

[auge8472]

I will merge the PR and will perform further tests afterwards.

I've tested the changes with an update from a stable version of the 2.4.x branch. I tested the queries for not breaking in case of a repeated change (in example: setting the field size of the e-mail-column to 255 thrice). But these tests were carried out under laboratory conditions for every of the queries (with a PHP-script with the code for changing the table definitions only) and not in a existing forum that was previously upgraded to an intermediate release version. Nevertheless I don't expect the queries to break anything.