From e05cd97b8af6ebda0080eec40018207d0c78acbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Wed, 29 Jul 2015 05:58:45 +0300 Subject: [PATCH] MDEV-8524: Improve error messaging when there is duplicate key or foreign key names Added better error message that will be printed when foreign key constraint name in create table is not unique in database. --- .../suite/innodb/r/innodb-fk-warnings.result | 20 +++ .../suite/innodb/t/innodb-fk-warnings.test | 29 +++++ storage/innobase/dict/dict0crea.c | 117 +++++++++++++++++- storage/innobase/handler/ha_innodb.cc | 26 ++++ storage/innobase/include/ha_prototypes.h | 9 ++ storage/xtradb/dict/dict0crea.c | 117 +++++++++++++++++- storage/xtradb/handler/ha_innodb.cc | 25 ++++ storage/xtradb/include/ha_prototypes.h | 9 ++ 8 files changed, 350 insertions(+), 2 deletions(-) create mode 100644 mysql-test/suite/innodb/r/innodb-fk-warnings.result create mode 100644 mysql-test/suite/innodb/t/innodb-fk-warnings.test diff --git a/mysql-test/suite/innodb/r/innodb-fk-warnings.result b/mysql-test/suite/innodb/r/innodb-fk-warnings.result new file mode 100644 index 00000000000..c64cb3024b5 --- /dev/null +++ b/mysql-test/suite/innodb/r/innodb-fk-warnings.result @@ -0,0 +1,20 @@ +CREATE TABLE t1 ( +id int(11) NOT NULL PRIMARY KEY, +a int(11) NOT NULL, +b int(11) NOT NULL, +c int not null, +CONSTRAINT test FOREIGN KEY (b) REFERENCES t1 (id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; +CREATE TABLE t2 ( +id int(11) NOT NULL PRIMARY KEY, +a int(11) NOT NULL, +b int(11) NOT NULL, +c int not null, +CONSTRAINT test FOREIGN KEY (b) REFERENCES t2 (id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; +ERROR HY000: Can't create table 'test.t2' (errno: 121) +show warnings; +Level Code Message +Warning 121 InnoDB: foreign key constraint name `test/test` already exists on data dictionary. Foreign key constraint names need to be unique in database. Error in foreign key definition: CONSTRAINT `test` FOREIGN KEY (`b`) REFERENCES `test`.`t2` (`id`). +Error 1005 Can't create table 'test.t2' (errno: 121) +drop table t1; diff --git a/mysql-test/suite/innodb/t/innodb-fk-warnings.test b/mysql-test/suite/innodb/t/innodb-fk-warnings.test new file mode 100644 index 00000000000..e12a0ecf820 --- /dev/null +++ b/mysql-test/suite/innodb/t/innodb-fk-warnings.test @@ -0,0 +1,29 @@ +--source include/have_innodb.inc + +# +# MDEV-8524: Improve error messaging when there is duplicate key or foreign key names +# +CREATE TABLE t1 ( + id int(11) NOT NULL PRIMARY KEY, + a int(11) NOT NULL, + b int(11) NOT NULL, + c int not null, + CONSTRAINT test FOREIGN KEY (b) REFERENCES t1 (id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +# +# Below create table fails because constraint name test +# is reserved for above table. +# +--error 1005 +CREATE TABLE t2 ( + id int(11) NOT NULL PRIMARY KEY, + a int(11) NOT NULL, + b int(11) NOT NULL, + c int not null, + CONSTRAINT test FOREIGN KEY (b) REFERENCES t2 (id) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +show warnings; + +drop table t1; diff --git a/storage/innobase/dict/dict0crea.c b/storage/innobase/dict/dict0crea.c index ac8a1eac03c..7a52aff3ec3 100644 --- a/storage/innobase/dict/dict0crea.c +++ b/storage/innobase/dict/dict0crea.c @@ -1419,6 +1419,91 @@ dict_create_add_foreign_field_to_dictionary( table, foreign, trx)); } +/********************************************************************//** +Construct foreign key constraint defintion from data dictionary information. +*/ +static +char* +dict_foreign_def_get( + dict_foreign_t* foreign,/*!< in: foreign */ + trx_t* trx) /*!< in: trx */ +{ + char* fk_def = mem_heap_alloc(foreign->heap, 4*1024); + const char* tbname; + char tablebuf[MAX_TABLE_NAME_LEN + 1] = ""; + int i; + + tbname = dict_remove_db_name(foreign->id); + innobase_convert_name(tablebuf, MAX_TABLE_NAME_LEN, + tbname, strlen(tbname), trx->mysql_thd, FALSE); + + sprintf(fk_def, + (char *)"CONSTRAINT %s FOREIGN KEY (", (char *)tablebuf); + + for(i = 0; i < foreign->n_fields; i++) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->foreign_col_names[i], + strlen(foreign->foreign_col_names[i]), + trx->mysql_thd, FALSE); + strcat(fk_def, buf); + if (i < foreign->n_fields-1) { + strcat(fk_def, (char *)","); + } + } + + strcat(fk_def,(char *)") REFERENCES "); + + innobase_convert_name(tablebuf, MAX_TABLE_NAME_LEN, + foreign->referenced_table_name, + strlen(foreign->referenced_table_name), + trx->mysql_thd, TRUE); + + strcat(fk_def, tablebuf); + strcat(fk_def, " ("); + + for(i = 0; i < foreign->n_fields; i++) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->referenced_col_names[i], + strlen(foreign->referenced_col_names[i]), + trx->mysql_thd, FALSE); + strcat(fk_def, buf); + if (i < foreign->n_fields-1) { + strcat(fk_def, (char *)","); + } + } + strcat(fk_def, (char *)")"); + + return fk_def; +} + +/********************************************************************//** +Convert foreign key column names from data dictionary to SQL-layer. +*/ +static +void +dict_foreign_def_get_fields( + dict_foreign_t* foreign,/*!< in: foreign */ + trx_t* trx, /*!< in: trx */ + char** field, /*!< out: foreign column */ + char** field2, /*!< out: referenced column */ + int col_no) /*!< in: column number */ +{ + *field = mem_heap_alloc(foreign->heap, MAX_TABLE_NAME_LEN+1); + *field2 = mem_heap_alloc(foreign->heap, MAX_TABLE_NAME_LEN+1); + + innobase_convert_name(*field, MAX_TABLE_NAME_LEN, + foreign->foreign_col_names[col_no], + strlen(foreign->foreign_col_names[col_no]), + trx->mysql_thd, FALSE); + + innobase_convert_name(*field, MAX_TABLE_NAME_LEN, + foreign->referenced_col_names[col_no], + strlen(foreign->referenced_col_names[col_no]), + trx->mysql_thd, FALSE); +} + /********************************************************************//** Add a single foreign key definition to the data dictionary tables in the database. We also generate names to constraints that were not named by the @@ -1501,6 +1586,22 @@ dict_create_add_foreign_to_dictionary( if (error != DB_SUCCESS) { + if (error == DB_DUPLICATE_KEY) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + char* fk_def; + + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->id, strlen(foreign->id), trx->mysql_thd, FALSE); + + fk_def = dict_foreign_def_get(foreign, trx); + + ib_push_warning(trx, error, (const char *)"InnoDB: foreign key constraint name %s " + "already exists on data dictionary." + " Foreign key constraint names need to be unique in database." + " Error in foreign key definition: %s.", + buf, fk_def); + } + return(error); } @@ -1509,6 +1610,20 @@ dict_create_add_foreign_to_dictionary( i, table, foreign, trx); if (error != DB_SUCCESS) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + char* field=NULL; + char* field2=NULL; + char* fk_def; + + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->id, strlen(foreign->id), trx->mysql_thd, FALSE); + fk_def = dict_foreign_def_get(foreign, trx); + dict_foreign_def_get_fields(foreign, trx, &field, &field2, i); + + ib_push_warning(trx, error, + (const char *)"InnoDB: Error adding foreign key constraint name %s fields %s or %s to the dictionary." + " Error in foreign key definition: %s.", + buf, i+1, fk_def); return(error); } @@ -1593,7 +1708,7 @@ dict_create_add_foreigns_to_dictionary( foreign = UT_LIST_GET_NEXT(foreign_list, foreign)) { error = dict_create_add_foreign_to_dictionary(&number, table, - foreign, trx); + foreign, trx); if (error != DB_SUCCESS) { diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index c97b5bfadc2..2bdbdf7a19f 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -12304,3 +12304,29 @@ ib_warn_row_too_big(const dict_table_t* table) " ROW_FORMAT=COMPRESSED ": "" , prefix ? DICT_MAX_FIXED_COL_LEN : 0); } + +/********************************************************************//** +Helper function to push warnings from InnoDB internals to SQL-layer. */ +extern "C" UNIV_INTERN +void +ib_push_warning( + trx_t* trx, /*!< in: trx */ + ulint error, /*!< in: error code to push as warning */ + const char *format,/*!< in: warning message */ + ...) +{ + va_list args; + THD *thd = (THD *)trx->mysql_thd; + char *buf; +#define MAX_BUF_SIZE 4*1024 + + va_start(args, format); + buf = (char *)my_malloc(MAX_BUF_SIZE, MYF(MY_WME)); + vsprintf(buf,format, args); + + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + convert_error_code_to_mysql(error, 0, thd), + buf); + my_free(buf); + va_end(args); +} diff --git a/storage/innobase/include/ha_prototypes.h b/storage/innobase/include/ha_prototypes.h index 3859b45e8d1..a69d3d126a0 100644 --- a/storage/innobase/include/ha_prototypes.h +++ b/storage/innobase/include/ha_prototypes.h @@ -326,5 +326,14 @@ innobase_convert_to_filename_charset( const char* from, /* in: identifier to convert */ ulint len); /* in: length of 'to', in bytes */ +/********************************************************************//** +Helper function to push warnings from InnoDB internals to SQL-layer. */ +UNIV_INTERN +void +ib_push_warning( + trx_t* trx, /*!< in: trx */ + ulint error, /*!< in: error code to push as warning */ + const char *format,/*!< in: warning message */ + ...); #endif diff --git a/storage/xtradb/dict/dict0crea.c b/storage/xtradb/dict/dict0crea.c index c88979b20f9..71a00ca8d0f 100644 --- a/storage/xtradb/dict/dict0crea.c +++ b/storage/xtradb/dict/dict0crea.c @@ -1626,6 +1626,91 @@ dict_create_add_foreign_field_to_dictionary( table, foreign, trx)); } +/********************************************************************//** +Construct foreign key constraint defintion from data dictionary information. +*/ +static +char* +dict_foreign_def_get( + dict_foreign_t* foreign,/*!< in: foreign */ + trx_t* trx) /*!< in: trx */ +{ + char* fk_def = mem_heap_alloc(foreign->heap, 4*1024); + const char* tbname; + char tablebuf[MAX_TABLE_NAME_LEN + 1] = ""; + int i; + + tbname = dict_remove_db_name(foreign->id); + innobase_convert_name(tablebuf, MAX_TABLE_NAME_LEN, + tbname, strlen(tbname), trx->mysql_thd, FALSE); + + sprintf(fk_def, + (char *)"CONSTRAINT %s FOREIGN KEY (", (char *)tablebuf); + + for(i = 0; i < foreign->n_fields; i++) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->foreign_col_names[i], + strlen(foreign->foreign_col_names[i]), + trx->mysql_thd, FALSE); + strcat(fk_def, buf); + if (i < foreign->n_fields-1) { + strcat(fk_def, (char *)","); + } + } + + strcat(fk_def,(char *)") REFERENCES "); + + innobase_convert_name(tablebuf, MAX_TABLE_NAME_LEN, + foreign->referenced_table_name, + strlen(foreign->referenced_table_name), + trx->mysql_thd, TRUE); + + strcat(fk_def, tablebuf); + strcat(fk_def, " ("); + + for(i = 0; i < foreign->n_fields; i++) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->referenced_col_names[i], + strlen(foreign->referenced_col_names[i]), + trx->mysql_thd, FALSE); + strcat(fk_def, buf); + if (i < foreign->n_fields-1) { + strcat(fk_def, (char *)","); + } + } + strcat(fk_def, (char *)")"); + + return fk_def; +} + +/********************************************************************//** +Convert foreign key column names from data dictionary to SQL-layer. +*/ +static +void +dict_foreign_def_get_fields( + dict_foreign_t* foreign,/*!< in: foreign */ + trx_t* trx, /*!< in: trx */ + char** field, /*!< out: foreign column */ + char** field2, /*!< out: referenced column */ + int col_no) /*!< in: column number */ +{ + *field = mem_heap_alloc(foreign->heap, MAX_TABLE_NAME_LEN+1); + *field2 = mem_heap_alloc(foreign->heap, MAX_TABLE_NAME_LEN+1); + + innobase_convert_name(*field, MAX_TABLE_NAME_LEN, + foreign->foreign_col_names[col_no], + strlen(foreign->foreign_col_names[col_no]), + trx->mysql_thd, FALSE); + + innobase_convert_name(*field, MAX_TABLE_NAME_LEN, + foreign->referenced_col_names[col_no], + strlen(foreign->referenced_col_names[col_no]), + trx->mysql_thd, FALSE); +} + /********************************************************************//** Add a single foreign key definition to the data dictionary tables in the database. We also generate names to constraints that were not named by the @@ -1708,6 +1793,22 @@ dict_create_add_foreign_to_dictionary( if (error != DB_SUCCESS) { + if (error == DB_DUPLICATE_KEY) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + char* fk_def; + + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->id, strlen(foreign->id), trx->mysql_thd, FALSE); + + fk_def = dict_foreign_def_get(foreign, trx); + + ib_push_warning(trx, error, (const char *)"InnoDB: foreign key constraint name %s " + "already exists on data dictionary." + " Foreign key constraint names need to be unique in database." + " Error in foreign key definition: %s.", + buf, fk_def); + } + return(error); } @@ -1716,6 +1817,20 @@ dict_create_add_foreign_to_dictionary( i, table, foreign, trx); if (error != DB_SUCCESS) { + char buf[MAX_TABLE_NAME_LEN + 1] = ""; + char* field=NULL; + char* field2=NULL; + char* fk_def; + + innobase_convert_name(buf, MAX_TABLE_NAME_LEN, + foreign->id, strlen(foreign->id), trx->mysql_thd, FALSE); + fk_def = dict_foreign_def_get(foreign, trx); + dict_foreign_def_get_fields(foreign, trx, &field, &field2, i); + + ib_push_warning(trx, error, + (const char *)"InnoDB: Error adding foreign key constraint name %s fields %s or %s to the dictionary." + " Error in foreign key definition: %s.", + buf, i+1, fk_def); return(error); } @@ -1800,7 +1915,7 @@ dict_create_add_foreigns_to_dictionary( foreign = UT_LIST_GET_NEXT(foreign_list, foreign)) { error = dict_create_add_foreign_to_dictionary(&number, table, - foreign, trx); + foreign, trx); if (error != DB_SUCCESS) { diff --git a/storage/xtradb/handler/ha_innodb.cc b/storage/xtradb/handler/ha_innodb.cc index fa0081da436..6c4e9eedd7d 100644 --- a/storage/xtradb/handler/ha_innodb.cc +++ b/storage/xtradb/handler/ha_innodb.cc @@ -14107,3 +14107,28 @@ ha_innobase::idx_cond_push( DBUG_RETURN(NULL); } +/********************************************************************//** +Helper function to push warnings from InnoDB internals to SQL-layer. */ +extern "C" UNIV_INTERN +void +ib_push_warning( + trx_t* trx, /*!< in: trx */ + ulint error, /*!< in: error code to push as warning */ + const char *format,/*!< in: warning message */ + ...) +{ + va_list args; + THD *thd = (THD *)trx->mysql_thd; + char *buf; +#define MAX_BUF_SIZE 4*1024 + + va_start(args, format); + buf = (char *)my_malloc(MAX_BUF_SIZE, MYF(MY_WME)); + vsprintf(buf,format, args); + + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + convert_error_code_to_mysql(error, 0, thd), + buf); + my_free(buf); + va_end(args); +} diff --git a/storage/xtradb/include/ha_prototypes.h b/storage/xtradb/include/ha_prototypes.h index ec2ba77f784..ad0abb88ff6 100644 --- a/storage/xtradb/include/ha_prototypes.h +++ b/storage/xtradb/include/ha_prototypes.h @@ -353,5 +353,14 @@ innobase_convert_to_filename_charset( const char* from, /* in: identifier to convert */ ulint len); /* in: length of 'to', in bytes */ +/********************************************************************//** +Helper function to push warnings from InnoDB internals to SQL-layer. */ +UNIV_INTERN +void +ib_push_warning( + trx_t* trx, /*!< in: trx */ + ulint error, /*!< in: error code to push as warning */ + const char *format,/*!< in: warning message */ + ...); #endif