From bf050b1db219944ff836d5125f262ab0cf1a8a21 Mon Sep 17 00:00:00 2001 From: Alexey Botchkov Date: Thu, 6 Feb 2014 16:27:05 +0100 Subject: [PATCH] MDEV-4439 ALTER TABLE .. [ADD|DROP] FOREIGN KEY IF [NOT] EXISTS does not work if constraint name is not used. Patches for server and the Innodb engine. Server is fixed so it does nothing if no indexes left to alter. Innodb parser is fixed so it looks for the IF [NOT] EXISTS option in a string. Another change is that it uses the index name for the internal dictionary. Prior to that it only used the CONSTRAINT name for it. --- mysql-test/r/alter_table.result | 73 +++++++++++++++++++++++++- mysql-test/t/alter_table.test | 41 ++++++++++++++- sql/sql_table.cc | 84 +++++++++++++++++++++++------- storage/innobase/dict/dict0dict.cc | 54 ++++++++++++++++--- 4 files changed, 223 insertions(+), 29 deletions(-) diff --git a/mysql-test/r/alter_table.result b/mysql-test/r/alter_table.result index f714e6646de..207f6166fe0 100644 --- a/mysql-test/r/alter_table.result +++ b/mysql-test/r/alter_table.result @@ -1354,7 +1354,7 @@ CREATE TABLE t1 ( id INT(11) NOT NULL, x_param INT(11) DEFAULT NULL, PRIMARY KEY (id) -); +) ENGINE=MYISAM; ALTER TABLE t1 ADD COLUMN IF NOT EXISTS id INT, ADD COLUMN IF NOT EXISTS lol INT AFTER id; Warnings: @@ -1390,6 +1390,77 @@ t1 CREATE TABLE `t1` ( KEY `x_param1` (`x_param`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 DROP TABLE t1; +CREATE TABLE t1 ( +id INT(11) NOT NULL, +x_param INT(11) DEFAULT NULL, +PRIMARY KEY (id) +) ENGINE=INNODB; +CREATE TABLE t2 ( +id INT(11) NOT NULL) ENGINE=INNODB; +ALTER TABLE t1 ADD COLUMN IF NOT EXISTS id INT, +ADD COLUMN IF NOT EXISTS lol INT AFTER id; +Warnings: +Note 1060 Duplicate column name 'id' +ALTER TABLE t1 ADD COLUMN IF NOT EXISTS lol INT AFTER id; +Warnings: +Note 1060 Duplicate column name 'lol' +ALTER TABLE t1 DROP COLUMN IF EXISTS lol; +ALTER TABLE t1 DROP COLUMN IF EXISTS lol; +Warnings: +Note 1091 Can't DROP 'lol'; check that column/key exists +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); +Warnings: +Note 1061 Duplicate key name 'x_param' +ALTER TABLE t1 MODIFY IF EXISTS lol INT; +Warnings: +Note 1054 Unknown column 'lol' in 't1' +DROP INDEX IF EXISTS x_param ON t1; +DROP INDEX IF EXISTS x_param ON t1; +Warnings: +Note 1091 Can't DROP 'x_param'; check that column/key exists +CREATE INDEX IF NOT EXISTS x_param1 ON t1(x_param); +CREATE INDEX IF NOT EXISTS x_param1 ON t1(x_param); +Warnings: +Note 1061 Duplicate key name 'x_param1' +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `id` int(11) NOT NULL, + `x_param` int(11) DEFAULT NULL, + PRIMARY KEY (`id`), + KEY `x_param1` (`x_param`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS fk(id) REFERENCES t1(id); +ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS fk(id) REFERENCES t1(id); +Warnings: +Note 1061 Duplicate key name 'fk' +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS fk; +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS fk; +Warnings: +Note 1091 Can't DROP 'fk'; check that column/key exists +SHOW CREATE TABLE t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `id` int(11) NOT NULL, + KEY `fk` (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +ALTER TABLE t2 ADD FOREIGN KEY (id) REFERENCES t1(id); +ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS t2_ibfk_1(id) REFERENCES t1(id); +Warnings: +Note 1061 Duplicate key name 't2_ibfk_1' +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS t2_ibfk_1; +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS t2_ibfk_1; +Warnings: +Note 1091 Can't DROP 't2_ibfk_1'; check that column/key exists +SHOW CREATE TABLE t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `id` int(11) NOT NULL, + KEY `id` (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t2; +DROP TABLE t1; # # Bug#11938817 ALTER BEHAVIOR DIFFERENT THEN DOCUMENTED # diff --git a/mysql-test/t/alter_table.test b/mysql-test/t/alter_table.test index 2053e2d9d59..3050fc0378d 100644 --- a/mysql-test/t/alter_table.test +++ b/mysql-test/t/alter_table.test @@ -1258,7 +1258,7 @@ CREATE TABLE t1 ( id INT(11) NOT NULL, x_param INT(11) DEFAULT NULL, PRIMARY KEY (id) -); +) ENGINE=MYISAM; ALTER TABLE t1 ADD COLUMN IF NOT EXISTS id INT, ADD COLUMN IF NOT EXISTS lol INT AFTER id; @@ -1277,6 +1277,45 @@ CREATE INDEX IF NOT EXISTS x_param1 ON t1(x_param); SHOW CREATE TABLE t1; DROP TABLE t1; +CREATE TABLE t1 ( + id INT(11) NOT NULL, + x_param INT(11) DEFAULT NULL, + PRIMARY KEY (id) +) ENGINE=INNODB; + +CREATE TABLE t2 ( + id INT(11) NOT NULL) ENGINE=INNODB; + +ALTER TABLE t1 ADD COLUMN IF NOT EXISTS id INT, + ADD COLUMN IF NOT EXISTS lol INT AFTER id; +ALTER TABLE t1 ADD COLUMN IF NOT EXISTS lol INT AFTER id; +ALTER TABLE t1 DROP COLUMN IF EXISTS lol; +ALTER TABLE t1 DROP COLUMN IF EXISTS lol; + +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); +ALTER TABLE t1 MODIFY IF EXISTS lol INT; + +DROP INDEX IF EXISTS x_param ON t1; +DROP INDEX IF EXISTS x_param ON t1; +CREATE INDEX IF NOT EXISTS x_param1 ON t1(x_param); +CREATE INDEX IF NOT EXISTS x_param1 ON t1(x_param); +SHOW CREATE TABLE t1; + +ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS fk(id) REFERENCES t1(id); +ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS fk(id) REFERENCES t1(id); +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS fk; +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS fk; +SHOW CREATE TABLE t2; +ALTER TABLE t2 ADD FOREIGN KEY (id) REFERENCES t1(id); +ALTER TABLE t2 ADD FOREIGN KEY IF NOT EXISTS t2_ibfk_1(id) REFERENCES t1(id); +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS t2_ibfk_1; +ALTER TABLE t2 DROP FOREIGN KEY IF EXISTS t2_ibfk_1; +SHOW CREATE TABLE t2; + +DROP TABLE t2; +DROP TABLE t1; + --echo # --echo # Bug#11938817 ALTER BEHAVIOR DIFFERENT THEN DOCUMENTED --echo # diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 45d9c5dc091..bd43dfff478 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -5304,7 +5304,8 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) { alter_info->flags&= ~Alter_info::ALTER_ADD_COLUMN; if (alter_info->key_list.is_empty()) - alter_info->flags&= ~Alter_info::ALTER_ADD_INDEX; + alter_info->flags&= ~(Alter_info::ALTER_ADD_INDEX | + Alter_info::ADD_FOREIGN_KEY); } break; } @@ -5379,13 +5380,32 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) else /* Alter_drop::KEY */ { uint n_key; - for (n_key=0; n_key < table->s->keys; n_key++) + if (drop->type != Alter_drop::FOREIGN_KEY) { - if (my_strcasecmp(system_charset_info, - drop->name, table->key_info[n_key].name) == 0) + for (n_key=0; n_key < table->s->keys; n_key++) { - remove_drop= FALSE; - break; + if (my_strcasecmp(system_charset_info, + drop->name, table->key_info[n_key].name) == 0) + { + remove_drop= FALSE; + break; + } + } + } + else + { + List fk_child_key_list; + FOREIGN_KEY_INFO *f_key; + table->file->get_foreign_key_list(thd, &fk_child_key_list); + List_iterator fk_key_it(fk_child_key_list); + while ((f_key= fk_key_it++)) + { + if (my_strcasecmp(system_charset_info, f_key->foreign_id->str, + drop->name) == 0) + { + remove_drop= FALSE; + break; + } } } } @@ -5397,7 +5417,8 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) drop_it.remove(); if (alter_info->drop_list.is_empty()) alter_info->flags&= ~(Alter_info::ALTER_DROP_COLUMN | - Alter_info::ALTER_DROP_INDEX); + Alter_info::ALTER_DROP_INDEX | + Alter_info::DROP_FOREIGN_KEY); } } } @@ -5408,6 +5429,7 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) Key *key; List_iterator key_it(alter_info->key_list); uint n_key; + bool remove_key; const char *keyname; while ((key=key_it++)) { @@ -5424,24 +5446,48 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) if (keyname == NULL) continue; } - for (n_key=0; n_key < table->s->keys; n_key++) + remove_key= FALSE; + if (key->type != Key::FOREIGN_KEY) { - if (my_strcasecmp(system_charset_info, - keyname, table->key_info[n_key].name) == 0) + for (n_key=0; n_key < table->s->keys; n_key++) { - push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, - ER_DUP_KEYNAME, ER(ER_DUP_KEYNAME), keyname); - key_it.remove(); - if (key->type == Key::FOREIGN_KEY) + if (my_strcasecmp(system_charset_info, + keyname, table->key_info[n_key].name) == 0) { - /* ADD FOREIGN KEY appends two items. */ - key_it.remove(); + remove_key= TRUE; + break; } - if (alter_info->key_list.is_empty()) - alter_info->flags&= ~Alter_info::ALTER_ADD_INDEX; - break; } } + else + { + List fk_child_key_list; + FOREIGN_KEY_INFO *f_key; + table->file->get_foreign_key_list(thd, &fk_child_key_list); + List_iterator fk_key_it(fk_child_key_list); + while ((f_key= fk_key_it++)) + { + if (my_strcasecmp(system_charset_info, f_key->foreign_id->str, + key->name.str) == 0) + remove_key= TRUE; + break; + } + } + if (remove_key) + { + push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, + ER_DUP_KEYNAME, ER(ER_DUP_KEYNAME), key->name.str); + key_it.remove(); + if (key->type == Key::FOREIGN_KEY) + { + /* ADD FOREIGN KEY appends two items. */ + key_it.remove(); + } + if (alter_info->key_list.is_empty()) + alter_info->flags&= ~(Alter_info::ALTER_ADD_INDEX | + Alter_info::ADD_FOREIGN_KEY); + break; + } } } diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 1a1697b2ffc..d16296179b0 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -4238,18 +4238,45 @@ loop: goto loop; } + if (my_isspace(cs, *ptr)) { + ptr1 = dict_accept(cs, ptr, "IF", &success); + + if (success) { + if (!my_isspace(cs, *ptr1)) { + goto loop; + } + ptr1 = dict_accept(cs, ptr1, "NOT", &success); + if (!success) { + goto loop; + } + ptr1 = dict_accept(cs, ptr1, "EXISTS", &success); + if (!success) { + goto loop; + } + ptr = ptr1; + } + } + ptr = dict_accept(cs, ptr, "(", &success); if (!success) { - /* MySQL allows also an index id before the '('; we - skip it */ - ptr = dict_skip_word(cs, ptr, &success); + if (constraint_name) { + /* MySQL allows also an index id before the '('; we + skip it */ + ptr = dict_skip_word(cs, ptr, &success); + if (!success) { + dict_foreign_report_syntax_err( + name, start_of_latest_foreign, ptr); + return(DB_CANNOT_ADD_CONSTRAINT); + } + } + else { + while (my_isspace(cs, *ptr)) { + ptr++; + } - if (!success) { - dict_foreign_report_syntax_err( - name, start_of_latest_foreign, ptr); - - return(DB_CANNOT_ADD_CONSTRAINT); + ptr = dict_scan_id(cs, ptr, heap, + &constraint_name, FALSE, FALSE); } ptr = dict_accept(cs, ptr, "(", &success); @@ -4723,6 +4750,7 @@ dict_foreign_parse_drop_constraints( char* str; size_t len; const char* ptr; + const char* ptr1; const char* id; struct charset_info_st* cs; @@ -4773,6 +4801,16 @@ loop: goto syntax_error; } + ptr1 = dict_accept(cs, ptr, "IF", &success); + + if (success && my_isspace(cs, *ptr1)) { + ptr1 = dict_accept(cs, ptr1, "EXISTS", &success); + if (success) { + + ptr = ptr1; + } + } + ptr = dict_scan_id(cs, ptr, heap, &id, FALSE, TRUE); if (id == NULL) {