Details
-
Bug
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
-
10.3(EOL), 10.4(EOL), 10.5
-
None
Description
During DDL, if one partition requires table rebuild and other partition doesn't need rebuild then all partition should be forced to rebuild.
In ha_innobase::commit_inplace_alter_table() assumes that all partition does same operation
/* Apply the changes to the data dictionary tables, for all
|
partitions. */
|
|
|
for (inplace_alter_handler_ctx** pctx = ctx_array;
|
*pctx && !fail; pctx++) {
|
ha_innobase_inplace_ctx* ctx
|
= static_cast<ha_innobase_inplace_ctx*>(*pctx);
|
|
|
DBUG_ASSERT(new_clustered == ctx->need_rebuild());
|
|
|
fail = commit_set_autoinc(ha_alter_info, ctx, altered_table,
|
table);
|
Attachments
Issue Links
- relates to
-
-
- Closed
-
Activity
Why would ha_partition need to rebuild all partitions?
Because InnoDB assumes that? Why InnoDB does it?
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.
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.
Just adding the workaround patch for future reference workaround_MDEV-21832.patch
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.
Patch is in bb-10.3-MDEV-21832 and bb-10.4-MDEV-21832
Patch is in bb-10.4-
MDEV-21832