Commit 5cae401b authored by Aleksey Midenkov's avatar Aleksey Midenkov

MDEV-25555 Server crashes in tree_record_pos after INPLACE-recreating index on HEAP table

Drop and add same key is considered rename (look ALTER_RENAME_INDEX in
fill_alter_inplace_info()). But in this case order of keys may be
changed, because mysql_prepare_alter_table() yet does not know about
rename and treats 2 operations: drop and add.

In that case we disable inplace algorithm for such engines as Memory,
MyISAM and Aria with ALTER_INDEX_ORDER flag. These engines have no
specialized check_if_supported_inplace_alter() and default
handler::check_if_supported_inplace_alter() sees an unknown flag and
returns HA_ALTER_INPLACE_NOT_SUPPORTED.

ha_innobase::check_if_supported_inplace_alter() works differently and
inplace is not disabled (with the help of modified
INNOBASE_INPLACE_IGNORE). add_drop_v_cols fork was also tweaked as it
wrongly failed with MSG_UNSUPPORTED_ALTER_ONLINE_ON_VIRTUAL_COLUMN
when it seen ALTER_INDEX_ORDER.

No-op operation must be still no-op no matter of ALTER_INDEX_ORDER
presence, so we tweek its condition as well.
parent b3bdc1c1
...@@ -3389,5 +3389,15 @@ test.t1 check status OK ...@@ -3389,5 +3389,15 @@ test.t1 check status OK
drop table t1; drop table t1;
set @@default_storage_engine= @save_default_engine; set @@default_storage_engine= @save_default_engine;
# #
# MDEV-25555 Server crashes in tree_record_pos after INPLACE-recreating index on HEAP table
#
create table t1 (a int, key idx1(a), key idx2 using btree(a)) engine=memory;
alter table t1 rename index idx1 to idx3, algorithm=inplace;
delete from t1 where a = 10;
alter table t1 drop key idx3, add key idx1(a), algorithm=inplace;
ERROR 0A000: ALGORITHM=INPLACE is not supported for this operation. Try ALGORITHM=COPY
delete from t1 where a = 11;
drop table t1;
#
# End of 10.5 tests # End of 10.5 tests
# #
...@@ -2608,6 +2608,18 @@ check table t1; ...@@ -2608,6 +2608,18 @@ check table t1;
drop table t1; drop table t1;
set @@default_storage_engine= @save_default_engine; set @@default_storage_engine= @save_default_engine;
--echo #
--echo # MDEV-25555 Server crashes in tree_record_pos after INPLACE-recreating index on HEAP table
--echo #
create table t1 (a int, key idx1(a), key idx2 using btree(a)) engine=memory;
alter table t1 rename index idx1 to idx3, algorithm=inplace;
delete from t1 where a = 10;
--error ER_ALTER_OPERATION_NOT_SUPPORTED
alter table t1 drop key idx3, add key idx1(a), algorithm=inplace;
delete from t1 where a = 11;
# cleanup
drop table t1;
--echo # --echo #
--echo # End of 10.5 tests --echo # End of 10.5 tests
--echo # --echo #
...@@ -596,17 +596,17 @@ t1 CREATE TABLE `t1` ( ...@@ -596,17 +596,17 @@ t1 CREATE TABLE `t1` (
`a` int(11) DEFAULT NULL, `a` int(11) DEFAULT NULL,
UNIQUE KEY `db_row_hash_1` (`db_row_hash_1`), UNIQUE KEY `db_row_hash_1` (`db_row_hash_1`),
UNIQUE KEY `db_row_hash_2` (`db_row_hash_2`), UNIQUE KEY `db_row_hash_2` (`db_row_hash_2`),
UNIQUE KEY `d` (`d`) USING HASH,
UNIQUE KEY `e` (`e`), UNIQUE KEY `e` (`e`),
UNIQUE KEY `a` (`a`), UNIQUE KEY `a` (`a`)
UNIQUE KEY `d` (`d`) USING HASH
) ENGINE=MyISAM DEFAULT CHARSET=latin1 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
show keys from t1; show keys from t1;
Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment
t1 0 db_row_hash_1 1 db_row_hash_1 A NULL NULL NULL YES BTREE t1 0 db_row_hash_1 1 db_row_hash_1 A NULL NULL NULL YES BTREE
t1 0 db_row_hash_2 1 db_row_hash_2 A NULL NULL NULL YES BTREE t1 0 db_row_hash_2 1 db_row_hash_2 A NULL NULL NULL YES BTREE
t1 0 d 1 d A NULL NULL NULL YES HASH
t1 0 e 1 e A NULL NULL NULL YES BTREE t1 0 e 1 e A NULL NULL NULL YES BTREE
t1 0 a 1 a A NULL NULL NULL YES BTREE t1 0 a 1 a A NULL NULL NULL YES BTREE
t1 0 d 1 d A NULL NULL NULL YES HASH
alter table t1 add column clm1 blob unique,add column clm2 blob unique; alter table t1 add column clm1 blob unique,add column clm2 blob unique;
#try changing the name; #try changing the name;
alter table t1 change column clm1 clm_changed1 blob, change column clm2 clm_changed2 blob; alter table t1 change column clm1 clm_changed1 blob, change column clm2 clm_changed2 blob;
......
...@@ -786,6 +786,13 @@ typedef bool Log_func(THD*, TABLE*, bool, const uchar*, const uchar*); ...@@ -786,6 +786,13 @@ typedef bool Log_func(THD*, TABLE*, bool, const uchar*, const uchar*);
*/ */
#define ALTER_COLUMN_INDEX_LENGTH (1ULL << 60) #define ALTER_COLUMN_INDEX_LENGTH (1ULL << 60)
/**
Indicate that index order might have been changed. Disables inplace algorithm
by default (not for InnoDB).
*/
#define ALTER_INDEX_ORDER (1ULL << 61)
/* /*
Flags set in partition_flags when altering partitions Flags set in partition_flags when altering partitions
*/ */
......
...@@ -10623,6 +10623,8 @@ do_continue:; ...@@ -10623,6 +10623,8 @@ do_continue:;
create_info->options|=HA_CREATE_TMP_ALTER; create_info->options|=HA_CREATE_TMP_ALTER;
if (!(alter_info->flags & ALTER_ADD_INDEX) && !alter_ctx.modified_primary_key) if (!(alter_info->flags & ALTER_ADD_INDEX) && !alter_ctx.modified_primary_key)
create_info->options|= HA_SKIP_KEY_SORT; create_info->options|= HA_SKIP_KEY_SORT;
else
alter_info->flags|= ALTER_INDEX_ORDER;
create_info->alias= alter_ctx.table_name; create_info->alias= alter_ctx.table_name;
error= create_table_impl(thd, alter_ctx.db, alter_ctx.table_name, error= create_table_impl(thd, alter_ctx.db, alter_ctx.table_name,
alter_ctx.new_db, alter_ctx.tmp_name, alter_ctx.new_db, alter_ctx.tmp_name,
...@@ -10659,7 +10661,7 @@ do_continue:; ...@@ -10659,7 +10661,7 @@ do_continue:;
*/ */
if (!(ha_alter_info.handler_flags & if (!(ha_alter_info.handler_flags &
~(ALTER_COLUMN_ORDER | ALTER_RENAME_COLUMN))) ~(ALTER_COLUMN_ORDER | ALTER_RENAME_COLUMN | ALTER_INDEX_ORDER)))
{ {
/* /*
No-op ALTER, no need to call handler API functions. No-op ALTER, no need to call handler API functions.
...@@ -10674,6 +10676,9 @@ do_continue:; ...@@ -10674,6 +10676,9 @@ do_continue:;
Also note that we ignore the LOCK clause here. Also note that we ignore the LOCK clause here.
TODO don't create partitioning metadata in the first place TODO don't create partitioning metadata in the first place
TODO: Now case-change index name is treated as noop which is not quite
correct.
*/ */
table->file->ha_create_partitioning_metadata(alter_ctx.get_tmp_path(), table->file->ha_create_partitioning_metadata(alter_ctx.get_tmp_path(),
NULL, CHF_DELETE_FLAG); NULL, CHF_DELETE_FLAG);
......
...@@ -7205,7 +7205,8 @@ ha_connect::check_if_supported_inplace_alter(TABLE *altered_table, ...@@ -7205,7 +7205,8 @@ ha_connect::check_if_supported_inplace_alter(TABLE *altered_table,
ALTER_ADD_UNIQUE_INDEX | ALTER_ADD_UNIQUE_INDEX |
ALTER_DROP_UNIQUE_INDEX | ALTER_DROP_UNIQUE_INDEX |
ALTER_ADD_PK_INDEX | ALTER_ADD_PK_INDEX |
ALTER_DROP_PK_INDEX; ALTER_DROP_PK_INDEX |
ALTER_INDEX_ORDER;
alter_table_operations inplace_offline_operations= alter_table_operations inplace_offline_operations=
ALTER_COLUMN_TYPE_CHANGE_BY_ENGINE | ALTER_COLUMN_TYPE_CHANGE_BY_ENGINE |
......
...@@ -114,6 +114,7 @@ static const alter_table_operations INNOBASE_INPLACE_IGNORE ...@@ -114,6 +114,7 @@ static const alter_table_operations INNOBASE_INPLACE_IGNORE
| ALTER_VIRTUAL_GCOL_EXPR | ALTER_VIRTUAL_GCOL_EXPR
| ALTER_DROP_CHECK_CONSTRAINT | ALTER_DROP_CHECK_CONSTRAINT
| ALTER_RENAME | ALTER_RENAME
| ALTER_INDEX_ORDER
| ALTER_COLUMN_INDEX_LENGTH | ALTER_COLUMN_INDEX_LENGTH
| ALTER_CHANGE_INDEX_COMMENT; | ALTER_CHANGE_INDEX_COMMENT;
...@@ -2454,7 +2455,8 @@ ha_innobase::check_if_supported_inplace_alter( ...@@ -2454,7 +2455,8 @@ ha_innobase::check_if_supported_inplace_alter(
| ALTER_ADD_UNIQUE_INDEX | ALTER_ADD_UNIQUE_INDEX
*/ */
| ALTER_ADD_NON_UNIQUE_NON_PRIM_INDEX | ALTER_ADD_NON_UNIQUE_NON_PRIM_INDEX
| ALTER_DROP_NON_UNIQUE_NON_PRIM_INDEX); | ALTER_DROP_NON_UNIQUE_NON_PRIM_INDEX
| ALTER_INDEX_ORDER);
if (supports_instant) { if (supports_instant) {
flags &= ~(ALTER_DROP_STORED_COLUMN flags &= ~(ALTER_DROP_STORED_COLUMN
#if 0 /* MDEV-17468: remove check_v_col_in_order() and fix the code */ #if 0 /* MDEV-17468: remove check_v_col_in_order() and fix the code */
......
...@@ -12427,6 +12427,7 @@ my_core::enum_alter_inplace_result ha_rocksdb::check_if_supported_inplace_alter( ...@@ -12427,6 +12427,7 @@ my_core::enum_alter_inplace_result ha_rocksdb::check_if_supported_inplace_alter(
ALTER_ADD_NON_UNIQUE_NON_PRIM_INDEX | ALTER_ADD_NON_UNIQUE_NON_PRIM_INDEX |
ALTER_PARTITIONED | ALTER_PARTITIONED |
ALTER_ADD_UNIQUE_INDEX | ALTER_ADD_UNIQUE_INDEX |
ALTER_INDEX_ORDER |
ALTER_CHANGE_CREATE_OPTION)) { ALTER_CHANGE_CREATE_OPTION)) {
DBUG_RETURN(my_core::HA_ALTER_INPLACE_NOT_SUPPORTED); DBUG_RETURN(my_core::HA_ALTER_INPLACE_NOT_SUPPORTED);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment