Hello Sergei, On 2/25/24 00:51, Sergei Golubchik wrote:

Hi, Alexander,

On Feb 24, Alexander Barkov wrote:

...

revision-id: f22662c36b1 (mariadb-10.11.7-6-gf22662c36b1) parent(s): 656f8867720 author: Alexander Barkov committer: Alexander Barkov timestamp: 2024-02-15 18:11:03 +0400 message:

MDEV-33442 REPAIR TABLE corrupts UUIDs

Looks quite good.

A question: you removed a hard-coded check for MYSQL_TYPE_NEWDECIMAL, but I don't see that you added Type_handler_olddecimal::type_handler_for_implicit_upgrade()

The old decimal does not implicitly upgrade to the new decimal. IIRC, some old decimal values do not convert to the new decimal. So one should use a manual ALTER to get rid of the old decimal. The hard-coded test was for the new decimal. if (!table->s->mysql_version) { /* check for bad DECIMAL field */ for (field= table->field; (*field); field++) { if ((*field)->type() == MYSQL_TYPE_NEWDECIMAL) { Note, mysql_version is written to FRM starting from 5.0.3. So this meant "force upgrade if FRM < 5.0.3 and the new decimal found". The above code looked redundant, because handler::ha_check_for_upgrade() has this piece of the code, where I added a comment: + /* + True VARCHAR appeared in MySQL-5.0.3. + If the FRM is older than 5.0.3, force alter even if the check_old_type() + call above did not find data types that want upgrade. + */ if (table->s->frm_version < FRM_VER_TRUE_VARCHAR) return HA_ADMIN_NEEDS_ALTER; It means "force upgrade if FRM < 5.0.3" (no matter with or without the new decimal).

A suggestion, rename Column_definition_implicit_upgrade(). You've changed its semantics, it's good to make sure that the code using old Column_definition_implicit_upgrade() (added e.g. in 10.6 and merged up) won't compile.

This is a good idea. Do you have ideas about the new name? I'm thinking of these two names: Column_definition_implicitly_upgrade() Column_definition_upgrade_implicitly() Thanks!

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org