Commit 08421a83 authored by Martin Skold's avatar Martin Skold

bug #31231 mysql_alter_table() tries to drop a non-existing table

bug#31233 mysql_alter_table() fails to drop UNIQUE KEY

mysql-test/suite/ndb/r/ndb_alter_table.result:
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: added test cases
mysql-test/suite/ndb/t/ndb_alter_table.test:
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: added test cases
sql/ha_ndbcluster.cc:
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY: Removed check for non-pk
  tables, not needed when mysql_alter_table checks apropriate flags
sql/mysql_priv.h:
  bug #31231  mysql_alter_table() tries to drop a non-existing table: added FRM_ONLY
  flag
sql/sql_table.cc:
  bug #31231  mysql_alter_table() tries to drop a non-existing table
  Don't invoke handler for tables defined with FRM_ONLY flag.
  bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
  When a table is defined without an explicit primary key
  mysql will choose the first found unique index defined over
  non-nullable fields (if such an index exists). This means
  that if such an index is added (the first) or dropped (the last)
  through an alter table, this equals adding or dropping a primary key.
  The implementation for on-line add/drop index did not consider
  this semantics. This patch ensures that only handlers with the
   correctly defined flags (see handler.h for explanation of the flags):
  HA_ONLINE_ADD_PK_INDEX
  HA_ONLINE_ADD_PK_INDEX_NO_WRITES
  HA_ONLINE_DROP_PK_INDEX
  HA_ONLINE_DROP_PK_INDEX_NO_WRITES
  are invoked for such on-line operations. All others handlers must
  perform a full (offline) alter table.
parent 30849acf
...@@ -354,16 +354,52 @@ select * from t1 where a = 12; ...@@ -354,16 +354,52 @@ select * from t1 where a = 12;
a b c a b c
12 403 NULL 12 403 NULL
drop table t1; drop table t1;
create table t1 (a int not null, b varchar(10)) engine=ndb; create table t1(a int not null) engine=ndb;
show index from t1; $PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment PRIMARY KEY($PK) - UniqueHashIndex
insert into t1 values (1),(2),(3);
alter table t1 add primary key (a); alter table t1 add primary key (a);
show index from t1; a Int PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment PRIMARY KEY(a) - UniqueHashIndex
t1 0 PRIMARY 1 a A 0 NULL NULL BTREE PRIMARY(a) - OrderedIndex
update t1 set a = 17 where a = 1;
select * from t1 order by a;
a
2
3
17
alter table t1 drop primary key; alter table t1 drop primary key;
show index from t1; $PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment PRIMARY KEY($PK) - UniqueHashIndex
update t1 set a = 1 where a = 17;
select * from t1 order by a;
a
1
2
3
drop table t1;
create table t1(a int not null) engine=ndb;
$PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
PRIMARY KEY($PK) - UniqueHashIndex
insert into t1 values (1),(2),(3);
create unique index pk on t1(a);
a Int PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
PRIMARY KEY(a) - UniqueHashIndex
update t1 set a = 17 where a = 1;
select * from t1 order by a;
a
2
3
17
alter table t1 drop index pk;
$PK Bigunsigned PRIMARY KEY DISTRIBUTION KEY AT=FIXED ST=MEMORY
PRIMARY KEY($PK) - UniqueHashIndex
update t1 set a = 1 where a = 17;
select * from t1 order by a;
a
1
2
3
drop table t1; drop table t1;
create table t1 (a int not null primary key, b int not null default 0, c varchar(254)) engine=ndb; create table t1 (a int not null primary key, b int not null default 0, c varchar(254)) engine=ndb;
show create table t1; show create table t1;
......
...@@ -411,13 +411,32 @@ select * from t1 where a = 12; ...@@ -411,13 +411,32 @@ select * from t1 where a = 12;
drop table t1; drop table t1;
# some other ALTER combinations # some other ALTER combinations
# add/drop pk # Check add/drop primary key (not supported on-line)
create table t1 (a int not null, b varchar(10)) engine=ndb; create table t1(a int not null) engine=ndb;
show index from t1; --exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
insert into t1 values (1),(2),(3);
alter table t1 add primary key (a); alter table t1 add primary key (a);
show index from t1; --exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
update t1 set a = 17 where a = 1;
select * from t1 order by a;
alter table t1 drop primary key; alter table t1 drop primary key;
show index from t1; --exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
update t1 set a = 1 where a = 17;
select * from t1 order by a;
drop table t1;
# bug#31233 mysql_alter_table() fails to drop UNIQUE KEY
create table t1(a int not null) engine=ndb;
--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
insert into t1 values (1),(2),(3);
create unique index pk on t1(a);
--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
update t1 set a = 17 where a = 1;
select * from t1 order by a;
alter table t1 drop index pk;
--exec $NDB_TOOLS_DIR/ndb_desc --no-defaults -d test t1 | grep PRIMARY
update t1 set a = 1 where a = 17;
select * from t1 order by a;
drop table t1; drop table t1;
# alter .. alter # alter .. alter
......
...@@ -9971,34 +9971,23 @@ bool ha_ndbcluster::check_if_incompatible_data(HA_CREATE_INFO *create_info, ...@@ -9971,34 +9971,23 @@ bool ha_ndbcluster::check_if_incompatible_data(HA_CREATE_INFO *create_info,
if (table_changes != IS_EQUAL_YES) if (table_changes != IS_EQUAL_YES)
DBUG_RETURN(COMPATIBLE_DATA_NO); DBUG_RETURN(COMPATIBLE_DATA_NO);
/**
* Changing from/to primary key
*
* This is _not_ correct, but check_if_incompatible_data-interface
* doesnt give more info, so I guess that we can't do any
* online add index if not using primary key
*
* This as mysql will handle a unique not null index as primary
* even wo/ user specifiying it... :-(
*
*/
if ((table_share->primary_key == MAX_KEY && pk) ||
(table_share->primary_key != MAX_KEY && !pk) ||
(table_share->primary_key == MAX_KEY && !pk && ai))
{
DBUG_RETURN(COMPATIBLE_DATA_NO);
}
/* Check that auto_increment value was not changed */ /* Check that auto_increment value was not changed */
if ((create_info->used_fields & HA_CREATE_USED_AUTO) && if ((create_info->used_fields & HA_CREATE_USED_AUTO) &&
create_info->auto_increment_value != 0) create_info->auto_increment_value != 0)
{
DBUG_PRINT("info", ("auto_increment value changed"));
DBUG_RETURN(COMPATIBLE_DATA_NO); DBUG_RETURN(COMPATIBLE_DATA_NO);
}
/* Check that row format didn't change */ /* Check that row format didn't change */
if ((create_info->used_fields & HA_CREATE_USED_AUTO) && if ((create_info->used_fields & HA_CREATE_USED_AUTO) &&
get_row_type() != create_info->row_type) get_row_type() != create_info->row_type)
{
DBUG_PRINT("info", ("row format changed"));
DBUG_RETURN(COMPATIBLE_DATA_NO); DBUG_RETURN(COMPATIBLE_DATA_NO);
}
DBUG_PRINT("info", ("new table seems compatible"));
DBUG_RETURN(COMPATIBLE_DATA_YES); DBUG_RETURN(COMPATIBLE_DATA_YES);
} }
......
...@@ -2240,6 +2240,7 @@ uint build_table_shadow_filename(char *buff, size_t bufflen, ...@@ -2240,6 +2240,7 @@ uint build_table_shadow_filename(char *buff, size_t bufflen,
#define FN_TO_IS_TMP (1 << 1) #define FN_TO_IS_TMP (1 << 1)
#define FN_IS_TMP (FN_FROM_IS_TMP | FN_TO_IS_TMP) #define FN_IS_TMP (FN_FROM_IS_TMP | FN_TO_IS_TMP)
#define NO_FRM_RENAME (1 << 2) #define NO_FRM_RENAME (1 << 2)
#define FRM_ONLY (1 << 3)
/* from hostname.cc */ /* from hostname.cc */
struct in_addr; struct in_addr;
......
...@@ -1835,8 +1835,9 @@ bool quick_rm_table(handlerton *base,const char *db, ...@@ -1835,8 +1835,9 @@ bool quick_rm_table(handlerton *base,const char *db,
if (my_delete(path,MYF(0))) if (my_delete(path,MYF(0)))
error= 1; /* purecov: inspected */ error= 1; /* purecov: inspected */
path[path_length - reg_ext_length]= '\0'; // Remove reg_ext path[path_length - reg_ext_length]= '\0'; // Remove reg_ext
DBUG_RETURN(ha_delete_table(current_thd, base, path, db, table_name, 0) || if (!(flags & FRM_ONLY))
error); error|= ha_delete_table(current_thd, base, path, db, table_name, 0);
DBUG_RETURN(error);
} }
/* /*
...@@ -5163,6 +5164,7 @@ mysql_discard_or_import_tablespace(THD *thd, ...@@ -5163,6 +5164,7 @@ mysql_discard_or_import_tablespace(THD *thd,
index_drop_count OUT The number of elements in the array. index_drop_count OUT The number of elements in the array.
index_add_buffer OUT An array of offsets into key_info_buffer. index_add_buffer OUT An array of offsets into key_info_buffer.
index_add_count OUT The number of elements in the array. index_add_count OUT The number of elements in the array.
candidate_key_count OUT The number of candidate keys in original table.
DESCRIPTION DESCRIPTION
'table' (first argument) contains information of the original 'table' (first argument) contains information of the original
...@@ -5193,7 +5195,8 @@ compare_tables(TABLE *table, ...@@ -5193,7 +5195,8 @@ compare_tables(TABLE *table,
enum_alter_table_change_level *need_copy_table, enum_alter_table_change_level *need_copy_table,
KEY **key_info_buffer, KEY **key_info_buffer,
uint **index_drop_buffer, uint *index_drop_count, uint **index_drop_buffer, uint *index_drop_count,
uint **index_add_buffer, uint *index_add_count) uint **index_add_buffer, uint *index_add_count,
uint *candidate_key_count)
{ {
Field **f_ptr, *field; Field **f_ptr, *field;
uint changes= 0, tmp; uint changes= 0, tmp;
...@@ -5208,6 +5211,9 @@ compare_tables(TABLE *table, ...@@ -5208,6 +5211,9 @@ compare_tables(TABLE *table,
create_info->varchar will be reset in mysql_prepare_create_table. create_info->varchar will be reset in mysql_prepare_create_table.
*/ */
bool varchar= create_info->varchar; bool varchar= create_info->varchar;
bool not_nullable= true;
DBUG_ENTER("compare_tables");
/* /*
Create a copy of alter_info. Create a copy of alter_info.
To compare the new and old table definitions, we need to "prepare" To compare the new and old table definitions, we need to "prepare"
...@@ -5225,24 +5231,21 @@ compare_tables(TABLE *table, ...@@ -5225,24 +5231,21 @@ compare_tables(TABLE *table,
*/ */
Alter_info tmp_alter_info(*alter_info, thd->mem_root); Alter_info tmp_alter_info(*alter_info, thd->mem_root);
uint db_options= 0; /* not used */ uint db_options= 0; /* not used */
DBUG_ENTER("compare_tables");
/* Create the prepared information. */ /* Create the prepared information. */
if (mysql_prepare_create_table(thd, create_info, if (mysql_prepare_create_table(thd, create_info,
&tmp_alter_info, &tmp_alter_info,
(table->s->tmp_table != NO_TMP_TABLE), (table->s->tmp_table != NO_TMP_TABLE),
&db_options, &db_options,
table->file, key_info_buffer, table->file, key_info_buffer,
&key_count, 0)) &key_count, 0))
DBUG_RETURN(1); DBUG_RETURN(1);
/* Allocate result buffers. */ /* Allocate result buffers. */
if (! (*index_drop_buffer= if (! (*index_drop_buffer=
(uint*) thd->alloc(sizeof(uint) * table->s->keys)) || (uint*) thd->alloc(sizeof(uint) * table->s->keys)) ||
! (*index_add_buffer= ! (*index_add_buffer=
(uint*) thd->alloc(sizeof(uint) * tmp_alter_info.key_list.elements))) (uint*) thd->alloc(sizeof(uint) * tmp_alter_info.key_list.elements)))
DBUG_RETURN(1); DBUG_RETURN(1);
/* /*
Some very basic checks. If number of fields changes, or the Some very basic checks. If number of fields changes, or the
handler, we need to run full ALTER TABLE. In the future handler, we need to run full ALTER TABLE. In the future
...@@ -5355,12 +5358,29 @@ compare_tables(TABLE *table, ...@@ -5355,12 +5358,29 @@ compare_tables(TABLE *table,
*/ */
*index_drop_count= 0; *index_drop_count= 0;
*index_add_count= 0; *index_add_count= 0;
*candidate_key_count= 0;
for (table_key= table->key_info; table_key < table_key_end; table_key++) for (table_key= table->key_info; table_key < table_key_end; table_key++)
{ {
KEY_PART_INFO *table_part; KEY_PART_INFO *table_part;
KEY_PART_INFO *table_part_end= table_key->key_part + table_key->key_parts; KEY_PART_INFO *table_part_end= table_key->key_part + table_key->key_parts;
KEY_PART_INFO *new_part; KEY_PART_INFO *new_part;
/*
Check if key is a candidate key, i.e. a unique index with no index
fields nullable, then key is either already primary key or could
be promoted to primary key if the original primary key is dropped.
Count all candidate keys.
*/
not_nullable= true;
for (table_part= table_key->key_part;
table_part < table_part_end;
table_part++)
{
not_nullable= not_nullable && (! table_part->field->maybe_null());
}
if ((table_key->flags & HA_NOSAME) && not_nullable)
(*candidate_key_count)++;
/* Search a new key with the same name. */ /* Search a new key with the same name. */
for (new_key= *key_info_buffer; new_key < new_key_end; new_key++) for (new_key= *key_info_buffer; new_key < new_key_end; new_key++)
{ {
...@@ -5986,13 +6006,16 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -5986,13 +6006,16 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
uint *index_drop_buffer; uint *index_drop_buffer;
uint index_add_count; uint index_add_count;
uint *index_add_buffer; uint *index_add_buffer;
uint candidate_key_count;
bool committed= 0; bool committed= 0;
bool no_pk;
DBUG_ENTER("mysql_alter_table"); DBUG_ENTER("mysql_alter_table");
LINT_INIT(index_add_count); LINT_INIT(index_add_count);
LINT_INIT(index_drop_count); LINT_INIT(index_drop_count);
LINT_INIT(index_add_buffer); LINT_INIT(index_add_buffer);
LINT_INIT(index_drop_buffer); LINT_INIT(index_drop_buffer);
LINT_INIT(candidate_key_count);
/* /*
Check if we attempt to alter mysql.slow_log or Check if we attempt to alter mysql.slow_log or
...@@ -6403,7 +6426,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6403,7 +6426,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
&need_copy_table_res, &need_copy_table_res,
&key_info_buffer, &key_info_buffer,
&index_drop_buffer, &index_drop_count, &index_drop_buffer, &index_drop_count,
&index_add_buffer, &index_add_count)) &index_add_buffer, &index_add_count,
&candidate_key_count))
goto err; goto err;
if (need_copy_table == ALTER_TABLE_METADATA_ONLY) if (need_copy_table == ALTER_TABLE_METADATA_ONLY)
...@@ -6437,20 +6461,40 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6437,20 +6461,40 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
DBUG_PRINT("info", ("index dropped: '%s'", key->name)); DBUG_PRINT("info", ("index dropped: '%s'", key->name));
if (key->flags & HA_NOSAME) if (key->flags & HA_NOSAME)
{ {
/* Unique key. Check for "PRIMARY". */ /*
if (! my_strcasecmp(system_charset_info, Unique key. Check for "PRIMARY".
key->name, primary_key_name)) or if dropping last unique key
*/
if ((uint) (key - table->key_info) == table->s->primary_key)
{ {
DBUG_PRINT("info", ("Dropping primary key"));
/* Primary key. */ /* Primary key. */
needed_online_flags|= HA_ONLINE_DROP_PK_INDEX; needed_online_flags|= HA_ONLINE_DROP_PK_INDEX;
needed_fast_flags|= HA_ONLINE_DROP_PK_INDEX_NO_WRITES; needed_fast_flags|= HA_ONLINE_DROP_PK_INDEX_NO_WRITES;
pk_changed++; pk_changed++;
candidate_key_count--;
} }
else else
{ {
KEY_PART_INFO *part_end= key->key_part + key->key_parts;
bool is_candidate_key= true;
/* Non-primary unique key. */ /* Non-primary unique key. */
needed_online_flags|= HA_ONLINE_DROP_UNIQUE_INDEX; needed_online_flags|= HA_ONLINE_DROP_UNIQUE_INDEX;
needed_fast_flags|= HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES; needed_fast_flags|= HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES;
/*
Check if all fields in key are declared
NOT NULL and adjust candidate_key_count
*/
for (KEY_PART_INFO *key_part= key->key_part;
key_part < part_end;
key_part++)
is_candidate_key=
(is_candidate_key &&
(! table->field[key_part->fieldnr-1]->maybe_null()));
if (is_candidate_key)
candidate_key_count--;
} }
} }
else else
...@@ -6460,7 +6504,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6460,7 +6504,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
needed_fast_flags|= HA_ONLINE_DROP_INDEX_NO_WRITES; needed_fast_flags|= HA_ONLINE_DROP_INDEX_NO_WRITES;
} }
} }
no_pk= ((table->s->primary_key == MAX_KEY) ||
(needed_online_flags & HA_ONLINE_DROP_PK_INDEX));
/* Check added indexes. */ /* Check added indexes. */
for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count; for (idx_p= index_add_buffer, idx_end_p= idx_p + index_add_count;
idx_p < idx_end_p; idx_p < idx_end_p;
...@@ -6470,14 +6515,38 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6470,14 +6515,38 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
DBUG_PRINT("info", ("index added: '%s'", key->name)); DBUG_PRINT("info", ("index added: '%s'", key->name));
if (key->flags & HA_NOSAME) if (key->flags & HA_NOSAME)
{ {
/* Unique key. Check for "PRIMARY". */ /* Unique key */
if (! my_strcasecmp(system_charset_info,
key->name, primary_key_name)) KEY_PART_INFO *part_end= key->key_part + key->key_parts;
bool is_candidate_key= true;
/*
Check if all fields in key are declared
NOT NULL
*/
for (KEY_PART_INFO *key_part= key->key_part;
key_part < part_end;
key_part++)
is_candidate_key=
(is_candidate_key &&
(! table->field[key_part->fieldnr]->maybe_null()));
/*
Check for "PRIMARY"
or if adding first unique key
defined on non-nullable fields
*/
if ((!my_strcasecmp(system_charset_info,
key->name, primary_key_name)) ||
(no_pk && candidate_key_count == 0 && is_candidate_key))
{ {
DBUG_PRINT("info", ("Adding primary key"));
/* Primary key. */ /* Primary key. */
needed_online_flags|= HA_ONLINE_ADD_PK_INDEX; needed_online_flags|= HA_ONLINE_ADD_PK_INDEX;
needed_fast_flags|= HA_ONLINE_ADD_PK_INDEX_NO_WRITES; needed_fast_flags|= HA_ONLINE_ADD_PK_INDEX_NO_WRITES;
pk_changed++; pk_changed++;
no_pk= false;
} }
else else
{ {
...@@ -6494,6 +6563,20 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6494,6 +6563,20 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
} }
} }
if ((candidate_key_count > 0) &&
(needed_online_flags & HA_ONLINE_DROP_PK_INDEX))
{
/*
Dropped primary key when there is some other unique
not null key that should be converted to primary key
*/
needed_online_flags|= HA_ONLINE_ADD_PK_INDEX;
needed_fast_flags|= HA_ONLINE_ADD_PK_INDEX_NO_WRITES;
pk_changed= 2;
}
DBUG_PRINT("info", ("needed_online_flags: 0x%lx, needed_fast_flags: 0x%lx",
needed_online_flags, needed_fast_flags));
/* /*
Online or fast add/drop index is possible only if Online or fast add/drop index is possible only if
the primary key is not added and dropped in the same statement. the primary key is not added and dropped in the same statement.
...@@ -6992,7 +7075,10 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6992,7 +7075,10 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
close_temporary_table(thd, new_table, 1, 1); close_temporary_table(thd, new_table, 1, 1);
} }
else else
VOID(quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP)); VOID(quick_rm_table(new_db_type, new_db, tmp_name,
create_info->frm_only
? FN_IS_TMP | FRM_ONLY
: FN_IS_TMP));
err: err:
/* /*
......
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