Thirunarayanan Balathandayuthapani added a comment - 2020-02-28 10:27 Patch is in bb-10.4- MDEV-21832

Sergei Golubchik added a comment - 2020-03-02 14:15 Why would ha_partition need to rebuild all partitions? Because InnoDB assumes that? Why InnoDB does it?

Marko Mäkelä added a comment - 2020-03-02 14:38 serg , since MySQL 5.6.10 (the first GA version), ha_innobase::commit_inplace_alter_table() does the internal commit for all partitions, so that the ALTER TABLE operation is atomic. That code is assuming that all partitions are handled in the same way (rebuild or not). It might be possible, but much trickier, to allow a subset of the partitions to be rebuilt.

Sergei Golubchik added a comment - 2020-03-02 16:00 Yes, but ha_partition is an upper-layer storage engine. It should not be tweaked to whatever limitations individual engines might have. If InnoDB wants all partitions to be rebuilt, it should try to handle this case internally, if possible. Without contaminating ha_partition with InnoDB-specific changes.

Thirunarayanan Balathandayuthapani added a comment - 2020-03-10 06:55 Just adding the workaround patch for future reference workaround_MDEV-21832.patch

Sergei Golubchik added a comment - 2020-03-26 19:02 here's fix I suggest diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 0434b891ffc..dfd622e0206 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -10099,7 +10099,8 @@ ha_partition::check_if_supported_inplace_alter(TABLE *altered_table, Alter_inplace_info *ha_alter_info) { uint index= 0; - enum_alter_inplace_result result= HA_ALTER_INPLACE_NO_LOCK; + enum_alter_inplace_result result; + alter_table_operations orig_ops; ha_partition_inplace_ctx *part_inplace_ctx; bool first_is_set= false; THD *thd= ha_thd(); @@ -10126,6 +10127,10 @@ ha_partition::check_if_supported_inplace_alter(TABLE *altered_table, if (!part_inplace_ctx->handler_ctx_array) DBUG_RETURN(HA_ALTER_ERROR); + do { + orig_ops= ha_alter_info->handler_flags; + result= HA_ALTER_INPLACE_NO_LOCK; + /* Set all to NULL, including the terminating one. */ for (index= 0; index <= m_tot_parts; index++) part_inplace_ctx->handler_ctx_array[index]= NULL; @@ -10154,6 +10159,8 @@ ha_partition::check_if_supported_inplace_alter(TABLE *altered_table, break; } + } while (orig_ops != ha_alter_info->handler_flags); + ha_alter_info->handler_ctx= part_inplace_ctx; /* To indicate for future inplace calls that there are several this is the partitioning part. The logic here — if ha_alter_info->handler_flags changes, we need to ask all partitions again, as it's basically a different ALTER TABLE now. And InnoDB, inside ha_innobase::check_if_supported_inplace_alter can set ALTER_RECREATE when needed.

Thirunarayanan Balathandayuthapani added a comment - 2020-03-27 11:03 Patch is in bb-10.3- MDEV-21832 and bb-10.4- MDEV-21832

Marko Mäkelä added a comment - 2020-03-30 08:30 OK to push to 10.3.