← Back to team overview

maria-developers team mailing list archive

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

 

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
> message:
> 
> 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
intentionally.

REPAIR TABLE was automatically switching to ALTER if these checks were
returning HA_ADMIN_NEEDS_ALTER.

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
`return HA_ADMIN_NEEDS_UPGRADE` with HA_ADMIN_NEEDS_ALTER.

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.

>        {
>          /* 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;
>        }

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups