maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander,

See a couple of questions below:

On Oct 04, Alexander Barkov wrote:
> revision-id: dcfc60ea2ee (mariadb-10.4.26-28-gdcfc60ea2ee)
> parent(s): 6c2c825e503
> author: Alexander Barkov
> committer: Alexander Barkov
> timestamp: 2022-10-03 10:32:42 +0400
> message:
> 
> MDEV-29481 mariadb-upgrade prints confusing statement
> 
> This is a new version of the patch instead of the reverted:
> 
>   MDEV-28727 ALTER TABLE ALGORITHM=NOCOPY does not work after upgrade

> diff --git a/mysql-test/main/alter_table_debug_aria.test b/mysql-test/main/alter_table_debug_aria.test
> new file mode 100644
> index 00000000000..22df57686e1
> --- /dev/null
> +++ b/mysql-test/main/alter_table_debug_aria.test
> @@ -0,0 +1,36 @@
> +--source include/have_debug.inc
> +--source include/have_maria.inc
> +
> +--echo #
> +--echo # Start of 10.4 tests
> +--echo #
> +
> +--echo #
> +--echo # MDEV-28727 ALTER TABLE ALGORITHM=NOCOPY does not work after upgrade
> +--echo #
> +
> +CREATE TABLE t1 (a DOUBLE NOT NULL, KEY(a)) ENGINE=ARIA;
> +ALTER TABLE t1 MODIFY a DOUBLE NOT NULL DEFAULT 10, ALGORITHM=NOCOPY;
> +DROP TABLE t1;
> +
> +CREATE TABLE t1 (a DOUBLE NOT NULL, KEY(a)) ENGINE=ARIA;
> +ALTER TABLE t1 MODIFY a DOUBLE NOT NULL DEFAULT 10, ALGORITHM=INSTANT;
> +DROP TABLE t1;
> +
> +SET SESSION debug_dbug='+d,emulate_pre_mdev_20704';
> +CREATE TABLE t1 (a DOUBLE NOT NULL, KEY(a)) ENGINE=ARIA;

Why did you add these debug-only tests with the emulate_pre_mdev_20704 hack,
when you have perfect non-debug tests with real pre-MDEV-20704 created tables ?

> +SET SESSION debug_dbug='';
> +--error ER_ALTER_OPERATION_NOT_SUPPORTED
> +ALTER TABLE t1 MODIFY a DOUBLE NOT NULL DEFAULT 10, ALGORITHM=INSTANT;
> +DROP TABLE t1;
> +
> +SET SESSION debug_dbug='+d,emulate_pre_mdev_20704';
> +CREATE TABLE t1 (a DOUBLE NOT NULL, KEY(a)) ENGINE=ARIA;
> +SET SESSION debug_dbug='';
> +--error ER_ALTER_OPERATION_NOT_SUPPORTED
> +ALTER TABLE t1 MODIFY a DOUBLE NOT NULL DEFAULT 10, ALGORITHM=NOCOPY;
> +DROP TABLE t1;
> +
> +--echo #
> +--echo # End of 10.4 tests
> +--echo #
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index 2436031200a..c192a4553a9 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -6748,11 +6753,13 @@ Compare_keys compare_keys_but_name(const KEY *table_key, const KEY *new_key,
>                                     const KEY *const new_pk,
>                                     const KEY *const old_pk)
>  {
> -  if (table_key->algorithm != new_key->algorithm)
> +  ulong key_flag_mask= HA_KEYFLAG_MASK |
> +                       table->file->key_pack_flags_supported();

I don't understand. You said that these KEY->flags do not affect what the
engine does. Why to compare them at all?

> +
> +  if ((table_key->flags & key_flag_mask) != (new_key->flags & key_flag_mask))
>      return Compare_keys::NotEqual;
>  
> -  if ((table_key->flags & HA_KEYFLAG_MASK) !=
> -      (new_key->flags & HA_KEYFLAG_MASK))
> +  if (table_key->algorithm != new_key->algorithm)
>      return Compare_keys::NotEqual;
>  
>    if (table_key->user_defined_key_parts != new_key->user_defined_key_parts)

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