⚡️ Add faster way to rename column on the same table

[Hipska]

Base information

Question	Answer
Related to a SourceForge thead / Another PR / Combodo ticket?	N/A
Type of change?	Enhancement

Objective

The process of moving data from one column to another could be simplified a lot when both are on the same table.

Proposed solution

Execute a RENAME COLUMN SQL statement instead of the following steps:

1. Create new column
2. Copy data from old to new column
3. Delete old column

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand digging in the code?

[Molkobain]

Careful, this is compatible with MySQL 8.0+, but not MySQL 5.7 which is still supported by iTop. You should stick to the CHANGE synthax:

Database	Version	CHANGE Compatibility	RENAME COLUMN Compatibility
MySQL	< 8.0	Yes	No
MySQL	8.0+	Yes	Yes
MariaDB	< 10.5.2	Yes	No
MariaDB	10.5.2+	Yes	Yes

[Hipska]

Oh, that's probably the reason I didn't create the PR back then.

Downside with CHANGE is that it needs the column definition, but there's a method for that, so fixed in 318a7e9.

[Molkobain]

CMDBSource::GetfIeldSpec() can return false, but I rather have an explicit error than hiding it. Good job.

[steffunky]

We already have some tests for this method in tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php, adding one testing this use case could be nice as @Molkobain already caught a regression in this method once
If you don't have time I'll try to add one sometime soon

[Hipska]

@Molkobain actually, the same method is doing the same without any checks:

iTop/setup/moduleinstaller.class.inc.php

Line 277 in c6039f4

$sSpec = CMDBSource::GetFieldSpec($sOrigTable, $sOrigColumn);

It will just never occur to return false because of the checks before.

[steffunky]

Works well on MariaDB 11.3.2 👍

[Hipska]

Funny, as that version isn't even supported by Combodo.

[rquetiez]

To take all the benefits out of this proposal, the table alteration algorithm must be at least inplace.

I've just conducted a quick study to check which DDL algorithm is used, and make sure that the syntax ALTER TABLE table CHANGE column... would not imply the infamous COPY algorithm.

Conclusion

The proposed implementation will be quick in any versions supported by iTop, excepted for the rare cases when the COPY algorithm is still used (e.g. table created with mariaDB < 10.3 and never rebuilt since the upgrade of the server).

Methodology

As an alternative to measuring the time spent on executing the alter table statement on a "huge" table, one can specify algorithm=inplace, so that the alter table statement would fail if the engine is not able to execute the INPLACE algorithm (or a quicker algorithm).

I've also tested with lock=none to document wether the operation involves locking or not (See Online DDL Performance and Concurrency)

alter table organization change code new_code varchar(255) default '' null, algorithm=inplace, lock=none;
alter table organization change code new_code varchar(255) default '' null, algorithm=instant;

Results

Database	Version	algorithm=inplace, lock=none	algorithm=instant
MySQL	5.7	Yes	n/a
MySQL	8.3	Yes	Yes
MariaDB	10.3	Yes	Yes