Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 10.0.10
-
Fix Version/s: 10.0.12
-
Component/s: None
-
Labels:None
Description
This probably was because all engines currently supporting partitioning do not support in place alter. However, this is wrong for engines that do support in place alter.
For instance when doing:
alter table partinp add index XID(id); alter table partinp drop index XID;
the data of CONNECT outward tables is modified (all rows are doubled)
Although it was possible to avoid this, calling check_if_supported_inplace_alter should be done to avoid unnecessary and time consuming operations.
Gliffy Diagrams
Attachments
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Sure enough the function exists in ha_partition. However, it does not call the engine used by the partition engine and, as it is compiled by default, returns unconditionally HA_ALTER_INPLACE_NOT_SUPPORTED while for CONNECT outward tables it should return HA_ALTER_INPLACE_NO_LOCK.
I am not sure this would be enough but it shows once again that the partition engine was written when only a few storage engines were supporting partitioning and is not really adapted for any new storage engines willing to support partitioning.
By the way, there are no example of partition tables in the current test suite because supporting partitioning by CONNECT is a work in progress and the current version of CONNECT does not support partitioning yet.
Okay, any way I can repeat this behavior? Because as far as I can see ha_partition::check_if_supported_inplace_alter() does call engine's check_if_supported_inplace_alter(), see:
enum_alter_inplace_result
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;
ha_partition_inplace_ctx *part_inplace_ctx;
bool first_is_set= false;
THD *thd= ha_thd();
DBUG_ENTER("ha_partition::check_if_supported_inplace_alter");
if (ha_alter_info->alter_info->flags == Alter_info::ALTER_PARTITION)
DBUG_RETURN(HA_ALTER_INPLACE_NO_LOCK);
part_inplace_ctx= new (thd->mem_root) ha_partition_inplace_ctx(thd, m_tot_parts);
if (!part_inplace_ctx)
DBUG_RETURN(HA_ALTER_ERROR);
part_inplace_ctx->handler_ctx_array= (inplace_alter_handler_ctx **)
thd->alloc(sizeof(inplace_alter_handler_ctx *) * (m_tot_parts + 1));
if (!part_inplace_ctx->handler_ctx_array)
DBUG_RETURN(HA_ALTER_ERROR);
ha_alter_info->handler_flags |= Alter_inplace_info::ALTER_PARTITIONED;
for (index= 0; index < m_tot_parts; index++)
{
enum_alter_inplace_result p_result=
m_file[index]->check_if_supported_inplace_alter(altered_table, ha_alter_info);
}
As you can see, it returns HA_ALTER_INPLACE_NO_LOCK for ALTER PARTITION, returns an error if memory allocation fails, and then calls check_if_supported_inplace_alter for every partition.
Sergei: On my machine, the complete code of ha_partition::check_if_supported_inplace_alter is:
enum_alter_inplace_result
ha_partition::check_if_supported_inplace_alter(TABLE *altered_table,
Alter_inplace_info *ha_alter_info)
{
#ifdef PARTITION_SUPPORTS_INPLACE_ALTER
uint index= 0;
enum_alter_inplace_result result= HA_ALTER_INPLACE_NO_LOCK;
ha_partition_inplace_ctx *part_inplace_ctx;
THD *thd= ha_thd();
#else
enum_alter_inplace_result result= HA_ALTER_INPLACE_NOT_SUPPORTED;
#endif
DBUG_ENTER("ha_partition::check_if_supported_inplace_alter");
/*
Support inplace change of KEY () -> KEY ALGORITHM = N ().
Any other change would set partition_changed in
prep_alter_part_table() in mysql_alter_table().
*/
if (ha_alter_info->alter_info->flags == Alter_info::ALTER_PARTITION)
DBUG_RETURN(HA_ALTER_INPLACE_NO_LOCK);
#ifndef PARTITION_SUPPORTS_INPLACE_ALTER
/*
Due to bug#14760210 partitions can be out-of-sync in case
commit_inplace_alter_table fails after the first partition.
Until we can either commit all partitions at the same time or
have an atomic recover on failure/crash we don't support any
inplace alter.
TODO: investigate what happens when indexes are out-of-sync
between partitions. If safe and possible to recover from,
then we could allow ADD/DROP INDEX.
*/
DBUG_RETURN(result);
#else
part_inplace_ctx=
new (thd->mem_root) ha_partition_inplace_ctx(thd, m_tot_parts);
if (!part_inplace_ctx)
DBUG_RETURN(HA_ALTER_ERROR);
part_inplace_ctx->handler_ctx_array= (inplace_alter_handler_ctx **)
thd->alloc(sizeof(inplace_alter_handler_ctx *) * m_tot_parts);
if (!part_inplace_ctx->handler_ctx_array)
DBUG_RETURN(HA_ALTER_ERROR);
for (index= 0; index < m_tot_parts; index++)
part_inplace_ctx->handler_ctx_array[index]= NULL;
for (index= 0; index < m_tot_parts; index++)
{
enum_alter_inplace_result p_result=
m_file[index]->check_if_supported_inplace_alter(altered_table,
ha_alter_info);
part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
if (p_result < result)
result= p_result;
if (result == HA_ALTER_ERROR)
break;
}
ha_alter_info->handler_ctx= part_inplace_ctx;
DBUG_RETURN(result);
#endif
}
It was compiled by default without PARTITION_SUPPORTS_INPLACE_ALTER defined. This is why the check_if_supported_inplace_alter function was not called for each partition. I shall recompile it after defining PARTITION_SUPPORTS_INPLACE_ALTER and let you know of what happens.
Apparently, recompiling the partition engine with PARTITION_SUPPORTS_INPLACE_ALTER defined solves the problems I add. In particular, DROP INDEX and CREATE INDEX that used to fail now works.
Note however the issue of this pre-compiler variable. Is it still used? If yes, it should be defined by default.
Please pull the latest 10.0 source tree. This PARTITION_SUPPORTS_INPLACE_ALTER was removed in 10.0.12.
I did try on
bzr+ssh://bertrandop@bazaar.launchpad.net/~maria-captains/maria/10.0-connect/
but it says:
No revisions or tags to pull.
Olivier Bertrand, as far as I can see, partition engine calls check_if_supported_inplace_alter(), it's right there in ha_partition::check_if_supported_inplace_alter() method.
Could you provide a complete test case for the buggy behavior that you observed? Starting from create table and everything. The connect test suite (in storage/connect/mysql-test/connect) contains no examples of partitioned connect tables.