MDEV-11757 KEY_BLOCK_SIZE strangeness when UNCOMPRESSing COMPRESSed InnoDB tables

in ALTER TABLE ... DROP KEY, ADD KEY, don't forget to compare old
and new keys' block sizes. If they differ - the key definition has changed.

6 files changed, 144 insertions, 1 deletions

diff --git a/mysql-test/suite/innodb/r/alter_key_block_size-11757.result b/mysql-test/suite/innodb/r/alter_key_block_size-11757.result
new file mode 100644
index 00000000000..400a3d0df3c
--- /dev/null
+++ b/mysql-test/suite/innodb/r/alter_key_block_size-11757.result
@@ -0,0 +1,47 @@
+set global innodb_file_format=barracuda;
+create table t1 (
+id1 bigint(20) not null,
+id2 bigint(20) not null,
+primary key (id1),
+unique key id2 (id2)
+) engine=innodb row_format=compressed key_block_size=8;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id1` bigint(20) NOT NULL,
+  `id2` bigint(20) NOT NULL,
+  PRIMARY KEY (`id1`),
+  UNIQUE KEY `id2` (`id2`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=8
+alter table t1 row_format=dynamic;
+Warnings:
+Warning	1478	InnoDB: ignoring KEY_BLOCK_SIZE=8 unless ROW_FORMAT=COMPRESSED.
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id1` bigint(20) NOT NULL,
+  `id2` bigint(20) NOT NULL,
+  PRIMARY KEY (`id1`),
+  UNIQUE KEY `id2` (`id2`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC KEY_BLOCK_SIZE=8
+alter table t1 key_block_size=0;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id1` bigint(20) NOT NULL,
+  `id2` bigint(20) NOT NULL,
+  PRIMARY KEY (`id1`) KEY_BLOCK_SIZE=8,
+  UNIQUE KEY `id2` (`id2`) KEY_BLOCK_SIZE=8
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC
+alter table t1 drop primary key, add primary key (id1),
+drop key id2, add unique (id2);
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `id1` bigint(20) NOT NULL,
+  `id2` bigint(20) NOT NULL,
+  PRIMARY KEY (`id1`),
+  UNIQUE KEY `id2` (`id2`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC
+drop table t1;
+set global innodb_file_format=default;
diff --git a/mysql-test/suite/innodb/t/alter_key_block_size-11757.test b/mysql-test/suite/innodb/t/alter_key_block_size-11757.test
new file mode 100644
index 00000000000..50a909870ba
--- /dev/null
+++ b/mysql-test/suite/innodb/t/alter_key_block_size-11757.test
@@ -0,0 +1,23 @@
+#
+# MDEV-11757 KEY_BLOCK_SIZE strangeness when UNCOMPRESSing COMPRESSed InnoDB tables
+#
+source include/have_innodb.inc;
+set global innodb_file_format=barracuda;
+
+create table t1 (
+  id1 bigint(20) not null,
+  id2 bigint(20) not null,
+  primary key (id1),
+  unique key id2 (id2)
+) engine=innodb row_format=compressed key_block_size=8;
+show create table t1;
+alter table t1 row_format=dynamic;
+show create table t1;
+alter table t1 key_block_size=0;
+show create table t1;
+alter table t1 drop primary key, add primary key (id1),
+               drop key id2, add unique (id2);
+show create table t1;
+drop table t1;
+
+set global innodb_file_format=default;
diff --git a/sql/handler.cc b/sql/handler.cc
index b261983ea0d..249d68fccb1 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -4209,7 +4209,7 @@ enum_alter_inplace_result
 handler::check_if_supported_inplace_alter(TABLE *altered_table,
                                           Alter_inplace_info *ha_alter_info)
 {
-  DBUG_ENTER("check_if_supported_alter");
+  DBUG_ENTER("handler::check_if_supported_inplace_alter");
 
   HA_CREATE_INFO *create_info= ha_alter_info->create_info;
 
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 5fde13c1f88..ef1b348d2ce 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -6429,6 +6429,9 @@ static bool fill_alter_inplace_info(THD *thd,
          new_key->user_defined_key_parts))
       goto index_changed;
 
+    if (table_key->block_size != new_key->block_size)
+      goto index_changed;
+
     if (engine_options_differ(table_key->option_struct, new_key->option_struct,
                               table->file->ht->index_options))
       goto index_changed;
diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
index dac01b8af89..730a63067ac 100644
--- a/storage/myisam/ha_myisam.cc
+++ b/storage/myisam/ha_myisam.cc
@@ -2179,6 +2179,74 @@ uint ha_myisam::checksum() const
 }
 
 
+enum_alter_inplace_result
+ha_myisam::check_if_supported_inplace_alter(TABLE *new_table,
+                                            Alter_inplace_info *alter_info)
+{
+  DBUG_ENTER("ha_myisam::check_if_supported_inplace_alter");
+
+  const uint readd_index= Alter_inplace_info::ADD_INDEX |
+                          Alter_inplace_info::DROP_INDEX;
+  const uint readd_unique= Alter_inplace_info::ADD_UNIQUE_INDEX |
+                           Alter_inplace_info::DROP_UNIQUE_INDEX;
+  const uint readd_pk= Alter_inplace_info::ADD_PK_INDEX |
+                       Alter_inplace_info::DROP_PK_INDEX;
+
+  const uint op= alter_info->handler_flags;
+
+  /*
+    ha_myisam::open() updates table->key_info->block_size to be the actual
+    MYI index block size, overwriting user-specified value (if any).
+    So, the server can not reliably detect whether ALTER TABLE changes
+    key_block_size or not, it might think the block size was changed,
+    when it wasn't, and in this case the server will recreate (drop+add)
+    the index unnecessary. Fix it.
+  */
+
+  if (table->s->keys == new_table->s->keys &&
+      ((op & readd_pk) == readd_pk ||
+       (op & readd_unique) == readd_unique ||
+       (op & readd_index) == readd_index))
+  {
+    for (uint i=0; i < table->s->keys; i++)
+    {
+      KEY *old_key= table->key_info + i;
+      KEY *new_key= new_table->key_info + i;
+
+      if (old_key->block_size == new_key->block_size)
+        DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED); // must differ somewhere else
+
+      if (new_key->block_size && new_key->block_size != old_key->block_size)
+        DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED); // really changed
+
+      /* any difference besides the block_size, and we give up */
+      if (old_key->key_length != new_key->key_length ||
+          old_key->flags != new_key->flags ||
+          old_key->user_defined_key_parts != new_key->user_defined_key_parts ||
+          old_key->algorithm != new_key->algorithm ||
+          strcmp(old_key->name, new_key->name))
+        DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+
+      for (uint j= 0; j < old_key->user_defined_key_parts; j++)
+      {
+        KEY_PART_INFO *old_kp= old_key->key_part + j;
+        KEY_PART_INFO *new_kp= new_key->key_part + j;
+        if (old_kp->offset != new_kp->offset ||
+            old_kp->null_offset != new_kp->null_offset ||
+            old_kp->length != new_kp->length ||
+            old_kp->fieldnr != new_kp->fieldnr ||
+            old_kp->key_part_flag != new_kp->key_part_flag ||
+            old_kp->type != new_kp->type ||
+            old_kp->null_bit != new_kp->null_bit)
+          DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+      }
+    }
+    alter_info->handler_flags &= ~(readd_pk | readd_unique | readd_index);
+  }
+  DBUG_RETURN(handler::check_if_supported_inplace_alter(new_table, alter_info));
+}
+
+
 bool ha_myisam::check_if_incompatible_data(HA_CREATE_INFO *info,
 					   uint table_changes)
 {
diff --git a/storage/myisam/ha_myisam.h b/storage/myisam/ha_myisam.h
index 63fb0ea5a2a..b132c955795 100644
--- a/storage/myisam/ha_myisam.h
+++ b/storage/myisam/ha_myisam.h
@@ -138,6 +138,8 @@ class ha_myisam: public handler
   int optimize(THD* thd, HA_CHECK_OPT* check_opt);
   int assign_to_keycache(THD* thd, HA_CHECK_OPT* check_opt);
   int preload_keys(THD* thd, HA_CHECK_OPT* check_opt);
+  enum_alter_inplace_result check_if_supported_inplace_alter(TABLE *new_table,
+                                            Alter_inplace_info *alter_info);
   bool check_if_incompatible_data(HA_CREATE_INFO *info, uint table_changes);
 #ifdef HAVE_QUERY_CACHE
   my_bool register_query_cache_table(THD *thd, char *table_key,