← Back to team overview

maria-developers team mailing list archive

Re: 06ce67c644b: MDEV-27653 long uniques don't work with unicode collations


Hello Sergei,

On 1/16/23 1:58 AM, Sergei Golubchik wrote:
Hi, Alexander,

On Jan 15, Alexander Barkov wrote:
revision-id: 06ce67c644b (mariadb-10.4.27-41-g06ce67c644b)
parent(s): 6cb84346e1b
author: Alexander Barkov
committer: Alexander Barkov
timestamp: 2023-01-10 18:27:16 +0400

MDEV-27653 long uniques don't work with unicode collations

diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc
index 213d77f8237..5dd19c877af 100644
--- a/sql/sql_admin.cc
+++ b/sql/sql_admin.cc
@@ -772,11 +772,14 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
        int check_for_upgrade= file->ha_check_for_upgrade(check_opt);
if (check_old_types == HA_ADMIN_NEEDS_ALTER ||
-          check_for_upgrade == HA_ADMIN_NEEDS_ALTER)
+          check_old_types == HA_ADMIN_NEEDS_UPGRADE ||
+          check_for_upgrade == HA_ADMIN_NEEDS_ALTER ||
+          check_for_upgrade == HA_ADMIN_NEEDS_UPGRADE)

eh. So old code was returning HA_ADMIN_NEEDS_ALTER actually quite

REPAIR TABLE was automatically switching to ALTER if these checks were

So it didn't matter if the message confusingly said "Please use REPAIR",
because it would've been ALTER internally anyway.

Meaning, I suspect, that the old code was fine and you didn't need to
change `case HA_ADMIN_NEEDS_ALTER` and didn't need to replace

All you needed to do was to return HA_ADMIN_NEEDS_ALTER from the
check_long_hash_compatibility() method and the rest would've likely
worked automatically. Or at least with much smaller changes.

REPAIR will not really work because of duplicate keys:

MariaDB [test]> repair table mdev27653_100422_text;
| Table | Op | Msg_type | Msg_text |
| test.mdev27653_100422_text | repair | Error | Duplicate entry 'ä' for key 'a' | | test.mdev27653_100422_text | repair | status | Operation failed |
2 rows in set (0.010 sec)

ALTER FORCE (without IGNORE) won't work either:

MariaDB [test]> alter table test.mdev27653_100422_text force;
ERROR 1062 (23000): Duplicate entry 'ä' for key 'a'

One needs to do ALTER IGNORE FORCE to fix this.

MariaDB [test]> alter ignore table test.mdev27653_100422_text force;
Query OK, 2 rows affected (0.003 sec)
Records: 2  Duplicates: 1  Warnings: 0

It would be totally confusing to print this error:
  Please do "REPAIR TABLE `t1`".

This error message is much better:
  Please do "ALTER TABLE `t1` FORCE"

It does not say about IGNORE, but at least it suggests
the correct verb statement.

The above changes are needed to suggest ALTER (rather than REPAIR)
in the error message text.

A 100% clear error message would be something like this:

Table rebuild required.
Please do "ALTER TABLE `t1` FORCE" or dump/reload to fix it!
If it returns duplicate key errors, get rid of duplicates and re-run ALTER again. Or do "ALTER IGNORE TABLE `t1` FORCE" if you want duplicates to be removed automatically (note, some data can be lost).

Should we add a new message like this?

          /* We use extra_open_options to be able to open crashed tables */
          thd->open_options|= extra_open_options;
-        result_code= admin_recreate_table(thd, table);
+        result_code= admin_recreate_table(thd, table) ? HA_ADMIN_FAILED :
+                                                        HA_ADMIN_OK;

good catch

          thd->open_options&= ~extra_open_options;
          goto send_result;

VP of MariaDB Server Engineering
and security@xxxxxxxxxxx