From b6fa4b11941549be8596f6720ec4f3c346a65bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 14 May 2010 16:31:44 +0300 Subject: [PATCH] Merge from mysql-5.1-innodb: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge fixes: Remove the MYSQL_VERSION_ID checks, because they only apply to the InnoDB Plugin. Fix potential race condition accessing trx->op_info and trx->detailed_error. ------------------------------------------------------------ revno: 3466 revision-id: marko.makela@oracle.com-20100514130815-ym7j7cfu88ro6km4 parent: marko.makela@oracle.com-20100514130228-n3n42nw7ht78k0wn committer: Marko Mäkelä branch nick: mysql-5.1-innodb2 timestamp: Fri 2010-05-14 16:08:15 +0300 message: Make the InnoDB FOREIGN KEY parser understand multi-statements. (Bug #48024) Also make InnoDB thinks that /*/ only starts a comment. (Bug #53644). This fixes the bugs in the InnoDB Plugin. ha_innodb.h: Use trx_query_string() instead of trx_query() when available (MySQL 5.1.42 or later). innobase_get_stmt(): New function, to retrieve the currently running SQL statement. struct trx_struct: Remove mysql_query_str. Use innobase_get_stmt() instead. dict_strip_comments(): Add and observe the parameter sql_length. Treat /*/ as the start of a comment. dict_create_foreign_constraints(), row_table_add_foreign_constraints(): Add the parameter sql_length. --- .../suite/innodb/r/innodb_bug48024.result | 10 ++++ .../suite/innodb/t/innodb_bug48024.test | 20 ++++++++ storage/innobase/dict/dict0dict.c | 51 ++++++++++++------- storage/innobase/handler/ha_innodb.cc | 29 +++++++++-- storage/innobase/handler/ha_innodb.h | 2 +- storage/innobase/include/dict0dict.h | 1 + storage/innobase/include/ha_prototypes.h | 12 ++++- storage/innobase/include/row0mysql.h | 1 + storage/innobase/include/trx0trx.h | 3 -- storage/innobase/row/row0mysql.c | 5 +- storage/innobase/trx/trx0i_s.c | 45 +++++++++++----- storage/innobase/trx/trx0trx.c | 2 - 12 files changed, 136 insertions(+), 45 deletions(-) create mode 100644 mysql-test/suite/innodb/r/innodb_bug48024.result create mode 100644 mysql-test/suite/innodb/t/innodb_bug48024.test diff --git a/mysql-test/suite/innodb/r/innodb_bug48024.result b/mysql-test/suite/innodb/r/innodb_bug48024.result new file mode 100644 index 00000000000..611923d2796 --- /dev/null +++ b/mysql-test/suite/innodb/r/innodb_bug48024.result @@ -0,0 +1,10 @@ +CREATE TABLE bug48024(a int PRIMARY KEY,b int NOT NULL,KEY(b)) ENGINE=InnoDB; +CREATE TABLE bug48024_b(b int PRIMARY KEY) ENGINE=InnoDB; +ALTER TABLE bug48024 /*/ADD CONSTRAINT FOREIGN KEY(c) REFERENCES(a),/*/ +ADD CONSTRAINT FOREIGN KEY(b) REFERENCES bug48024_b(b); +DROP TABLE bug48024,bug48024_b; +CREATE TABLE bug48024(a int PRIMARY KEY,b int NOT NULL,KEY(b)) ENGINE=InnoDB; +CREATE TABLE bug48024_b(b int PRIMARY KEY) ENGINE=InnoDB; +ALTER TABLE bug48024 /*/ADD CONSTRAINT FOREIGN KEY(c) REFERENCES(a),/*/ +ADD CONSTRAINT FOREIGN KEY(b) REFERENCES bug48024_b(b)| +DROP TABLE bug48024,bug48024_b; diff --git a/mysql-test/suite/innodb/t/innodb_bug48024.test b/mysql-test/suite/innodb/t/innodb_bug48024.test new file mode 100644 index 00000000000..00d76beb89d --- /dev/null +++ b/mysql-test/suite/innodb/t/innodb_bug48024.test @@ -0,0 +1,20 @@ +# Bug #48024 Innodb doesn't work with multi-statements + +--source include/have_innodb.inc + +CREATE TABLE bug48024(a int PRIMARY KEY,b int NOT NULL,KEY(b)) ENGINE=InnoDB; +CREATE TABLE bug48024_b(b int PRIMARY KEY) ENGINE=InnoDB; +# Bug #53644 InnoDB thinks that /*/ starts and ends a comment +ALTER TABLE bug48024 /*/ADD CONSTRAINT FOREIGN KEY(c) REFERENCES(a),/*/ +ADD CONSTRAINT FOREIGN KEY(b) REFERENCES bug48024_b(b); + +DROP TABLE bug48024,bug48024_b; + +delimiter |; +CREATE TABLE bug48024(a int PRIMARY KEY,b int NOT NULL,KEY(b)) ENGINE=InnoDB; +CREATE TABLE bug48024_b(b int PRIMARY KEY) ENGINE=InnoDB; +ALTER TABLE bug48024 /*/ADD CONSTRAINT FOREIGN KEY(c) REFERENCES(a),/*/ +ADD CONSTRAINT FOREIGN KEY(b) REFERENCES bug48024_b(b)| +delimiter ;| + +DROP TABLE bug48024,bug48024_b; diff --git a/storage/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c index ae3e7520b85..a298d785449 100644 --- a/storage/innobase/dict/dict0dict.c +++ b/storage/innobase/dict/dict0dict.c @@ -3023,25 +3023,28 @@ static char* dict_strip_comments( /*================*/ - const char* sql_string) /*!< in: SQL string */ + const char* sql_string, /*!< in: SQL string */ + size_t sql_length) /*!< in: length of sql_string */ { char* str; const char* sptr; + const char* eptr = sql_string + sql_length; char* ptr; /* unclosed quote character (0 if none) */ char quote = 0; - str = mem_alloc(strlen(sql_string) + 1); + str = mem_alloc(sql_length + 1); sptr = sql_string; ptr = str; for (;;) { scan_more: - if (*sptr == '\0') { + if (sptr >= eptr || *sptr == '\0') { +end_of_string: *ptr = '\0'; - ut_a(ptr <= str + strlen(sql_string)); + ut_a(ptr <= str + sql_length); return(str); } @@ -3060,30 +3063,35 @@ scan_more: || (sptr[0] == '-' && sptr[1] == '-' && sptr[2] == ' ')) { for (;;) { + if (++sptr >= eptr) { + goto end_of_string; + } + /* In Unix a newline is 0x0A while in Windows it is 0x0D followed by 0x0A */ - if (*sptr == (char)0x0A - || *sptr == (char)0x0D - || *sptr == '\0') { - + switch (*sptr) { + case (char) 0X0A: + case (char) 0x0D: + case '\0': goto scan_more; } - - sptr++; } } else if (!quote && *sptr == '/' && *(sptr + 1) == '*') { + sptr += 2; for (;;) { - if (*sptr == '*' && *(sptr + 1) == '/') { - - sptr += 2; - - goto scan_more; + if (sptr >= eptr) { + goto end_of_string; } - if (*sptr == '\0') { - + switch (*sptr) { + case '\0': goto scan_more; + case '*': + if (sptr[1] == '/') { + sptr += 2; + goto scan_more; + } } sptr++; @@ -3764,6 +3772,7 @@ dict_create_foreign_constraints( name before it: test.table2; the default database id the database of parameter name */ + size_t sql_length, /*!< in: length of sql_string */ const char* name, /*!< in: table full name in the normalized form database_name/table_name */ @@ -3778,7 +3787,7 @@ dict_create_foreign_constraints( ut_a(trx); ut_a(trx->mysql_thd); - str = dict_strip_comments(sql_string); + str = dict_strip_comments(sql_string, sql_length); heap = mem_heap_create(10000); err = dict_create_foreign_constraints_low( @@ -3811,6 +3820,7 @@ dict_foreign_parse_drop_constraints( dict_foreign_t* foreign; ibool success; char* str; + size_t len; const char* ptr; const char* id; FILE* ef = dict_foreign_err_file; @@ -3825,7 +3835,10 @@ dict_foreign_parse_drop_constraints( *constraints_to_drop = mem_heap_alloc(heap, 1000 * sizeof(char*)); - str = dict_strip_comments(*(trx->mysql_query_str)); + ptr = innobase_get_stmt(trx->mysql_thd, &len); + + str = dict_strip_comments(ptr, len); + ptr = str; ut_ad(mutex_own(&(dict_sys->mutex))); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 4a8d91cac11..12561c8c2a6 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -1133,6 +1133,23 @@ innobase_get_charset( return(thd_charset((THD*) mysql_thd)); } +/**********************************************************************//** +Determines the current SQL statement. +@return SQL statement string */ +extern "C" UNIV_INTERN +const char* +innobase_get_stmt( +/*==============*/ + void* mysql_thd, /*!< in: MySQL thread handle */ + size_t* length) /*!< out: length of the SQL statement */ +{ + LEX_STRING* stmt; + + stmt = thd_query_string((THD*) mysql_thd); + *length = stmt->length; + return(stmt->str); +} + #if defined (__WIN__) && defined (MYSQL_DYNAMIC_PLUGIN) extern MYSQL_PLUGIN_IMPORT MY_TMPDIR mysql_tmpdir_list; /*******************************************************************//** @@ -1465,7 +1482,6 @@ innobase_trx_allocate( trx = trx_allocate_for_mysql(); trx->mysql_thd = thd; - trx->mysql_query_str = thd_query(thd); innobase_trx_init(thd, trx); @@ -6632,6 +6648,8 @@ ha_innobase::create( /* Cache the value of innodb_file_format, in case it is modified by another thread while the table is being created. */ const ulint file_format = srv_file_format; + const char* stmt; + size_t stmt_len; DBUG_ENTER("ha_innobase::create"); @@ -6908,9 +6926,11 @@ ha_innobase::create( } } - if (*trx->mysql_query_str) { - error = row_table_add_foreign_constraints(trx, - *trx->mysql_query_str, norm_name, + stmt = innobase_get_stmt(thd, &stmt_len); + + if (stmt) { + error = row_table_add_foreign_constraints( + trx, stmt, stmt_len, norm_name, create_info->options & HA_LEX_CREATE_TMP_TABLE); error = convert_error_code_to_mysql(error, flags, NULL); @@ -7195,7 +7215,6 @@ innobase_drop_database( /* In the Windows plugin, thd = current_thd is always NULL */ trx = trx_allocate_for_mysql(); trx->mysql_thd = NULL; - trx->mysql_query_str = NULL; #else trx = innobase_trx_allocate(thd); #endif diff --git a/storage/innobase/handler/ha_innodb.h b/storage/innobase/handler/ha_innodb.h index 8a3e1ccff82..e5bc757ed72 100644 --- a/storage/innobase/handler/ha_innodb.h +++ b/storage/innobase/handler/ha_innodb.h @@ -231,7 +231,7 @@ the definitions are bracketed with #ifdef INNODB_COMPATIBILITY_HOOKS */ extern "C" { struct charset_info_st *thd_charset(MYSQL_THD thd); -char **thd_query(MYSQL_THD thd); +LEX_STRING *thd_query_string(MYSQL_THD thd); /** Get the file name of the MySQL binlog. * @return the name of the binlog file diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h index 79dcbb30de2..3a1bee4cd89 100644 --- a/storage/innobase/include/dict0dict.h +++ b/storage/innobase/include/dict0dict.h @@ -352,6 +352,7 @@ dict_create_foreign_constraints( name before it: test.table2; the default database id the database of parameter name */ + size_t sql_length, /*!< in: length of sql_string */ const char* name, /*!< in: table full name in the normalized form database_name/table_name */ diff --git a/storage/innobase/include/ha_prototypes.h b/storage/innobase/include/ha_prototypes.h index 9725ef05ad8..a9ee1d66b99 100644 --- a/storage/innobase/include/ha_prototypes.h +++ b/storage/innobase/include/ha_prototypes.h @@ -215,11 +215,21 @@ innobase_casedn_str( /**********************************************************************//** Determines the connection character set. @return connection character set */ +UNIV_INTERN struct charset_info_st* innobase_get_charset( /*=================*/ void* mysql_thd); /*!< in: MySQL thread handle */ - +/**********************************************************************//** +Determines the current SQL statement. +@return SQL statement string */ +UNIV_INTERN +const char* +innobase_get_stmt( +/*==============*/ + void* mysql_thd, /*!< in: MySQL thread handle */ + size_t* length) /*!< out: length of the SQL statement */ + __attribute__((nonnull)); /******************************************************************//** This function is used to find the storage length in bytes of the first n characters for prefix indexes using a multibyte character set. The function diff --git a/storage/innobase/include/row0mysql.h b/storage/innobase/include/row0mysql.h index d2a8734c61f..bf9cda1ba80 100644 --- a/storage/innobase/include/row0mysql.h +++ b/storage/innobase/include/row0mysql.h @@ -403,6 +403,7 @@ row_table_add_foreign_constraints( FOREIGN KEY (a, b) REFERENCES table2(c, d), table2 can be written also with the database name before it: test.table2 */ + size_t sql_length, /*!< in: length of sql_string */ const char* name, /*!< in: table full name in the normalized form database_name/table_name */ diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index 6872fb463c0..abd175d365b 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -560,9 +560,6 @@ struct trx_struct{ /*------------------------------*/ void* mysql_thd; /*!< MySQL thread handle corresponding to this trx, or NULL */ - char** mysql_query_str;/* pointer to the field in mysqld_thd - which contains the pointer to the - current SQL query string */ const char* mysql_log_file_name; /* if MySQL binlog is used, this field contains a pointer to the latest file diff --git a/storage/innobase/row/row0mysql.c b/storage/innobase/row/row0mysql.c index c900e33596a..9592de88346 100644 --- a/storage/innobase/row/row0mysql.c +++ b/storage/innobase/row/row0mysql.c @@ -2060,6 +2060,7 @@ row_table_add_foreign_constraints( FOREIGN KEY (a, b) REFERENCES table2(c, d), table2 can be written also with the database name before it: test.table2 */ + size_t sql_length, /*!< in: length of sql_string */ const char* name, /*!< in: table full name in the normalized form database_name/table_name */ @@ -2081,8 +2082,8 @@ row_table_add_foreign_constraints( trx_set_dict_operation(trx, TRX_DICT_OP_TABLE); - err = dict_create_foreign_constraints(trx, sql_string, name, - reject_fks); + err = dict_create_foreign_constraints(trx, sql_string, sql_length, + name, reject_fks); if (err == DB_SUCCESS) { /* Check that also referencing constraints are ok */ err = dict_load_foreigns(name, TRUE); diff --git a/storage/innobase/trx/trx0i_s.c b/storage/innobase/trx/trx0i_s.c index 570098d84ea..2e72940ea23 100644 --- a/storage/innobase/trx/trx0i_s.c +++ b/storage/innobase/trx/trx0i_s.c @@ -438,6 +438,10 @@ fill_trx_row( which to copy volatile strings */ { + const char* stmt; + size_t stmt_len; + const char* s; + row->trx_id = trx_get_id(trx); row->trx_started = (ib_time_t) trx->start_time; row->trx_state = trx_get_que_state_str(trx); @@ -458,20 +462,32 @@ fill_trx_row( row->trx_weight = (ullint) ut_conv_dulint_to_longlong(TRX_WEIGHT(trx)); - if (trx->mysql_thd != NULL) { - row->trx_mysql_thread_id - = thd_get_thread_id(trx->mysql_thd); - } else { + if (trx->mysql_thd == NULL) { /* For internal transactions e.g., purge and transactions being recovered at startup there is no associated MySQL thread data structure. */ row->trx_mysql_thread_id = 0; + row->trx_query = NULL; + goto thd_done; } - if (trx->mysql_query_str != NULL && *trx->mysql_query_str != NULL) { + row->trx_mysql_thread_id = thd_get_thread_id(trx->mysql_thd); + stmt = innobase_get_stmt(trx->mysql_thd, &stmt_len); - TRX_I_S_STRING_COPY(*trx->mysql_query_str, row->trx_query, - TRX_I_S_TRX_QUERY_MAX_LEN, cache); + if (stmt != NULL) { + + char query[TRX_I_S_TRX_QUERY_MAX_LEN + 1]; + + if (stmt_len > TRX_I_S_TRX_QUERY_MAX_LEN) { + stmt_len = TRX_I_S_TRX_QUERY_MAX_LEN; + } + + memcpy(query, stmt, stmt_len); + query[stmt_len] = '\0'; + + row->trx_query = ha_storage_put_memlim( + cache->storage, stmt, stmt_len + 1, + MAX_ALLOWED_FOR_STORAGE(cache)); if (row->trx_query == NULL) { @@ -482,9 +498,12 @@ fill_trx_row( row->trx_query = NULL; } - if (trx->op_info != NULL && trx->op_info[0] != '\0') { +thd_done: + s = trx->op_info; - TRX_I_S_STRING_COPY(trx->op_info, row->trx_operation_state, + if (s != NULL && s[0] != '\0') { + + TRX_I_S_STRING_COPY(s, row->trx_operation_state, TRX_I_S_TRX_OP_STATE_MAX_LEN, cache); if (row->trx_operation_state == NULL) { @@ -532,9 +551,11 @@ fill_trx_row( row->trx_foreign_key_checks = (ibool) trx->check_foreigns; - if (trx->detailed_error != NULL && trx->detailed_error[0] != '\0') { + s = trx->detailed_error; - TRX_I_S_STRING_COPY(trx->detailed_error, + if (s != NULL && s[0] != '\0') { + + TRX_I_S_STRING_COPY(s, row->trx_foreign_key_error, TRX_I_S_TRX_FK_ERROR_MAX_LEN, cache); @@ -543,7 +564,7 @@ fill_trx_row( return(FALSE); } } else { - row->trx_foreign_key_error = NULL; + row->trx_foreign_key_error = NULL; } row->trx_has_search_latch = (ibool) trx->has_search_latch; diff --git a/storage/innobase/trx/trx0trx.c b/storage/innobase/trx/trx0trx.c index 442037ad20a..c794671f7be 100644 --- a/storage/innobase/trx/trx0trx.c +++ b/storage/innobase/trx/trx0trx.c @@ -124,7 +124,6 @@ trx_create( trx->table_id = ut_dulint_zero; trx->mysql_thd = NULL; - trx->mysql_query_str = NULL; trx->active_trans = 0; trx->duplicates = 0; @@ -944,7 +943,6 @@ trx_commit_off_kernel( trx->rseg = NULL; trx->undo_no = ut_dulint_zero; trx->last_sql_stat_start.least_undo_no = ut_dulint_zero; - trx->mysql_query_str = NULL; ut_ad(UT_LIST_GET_LEN(trx->wait_thrs) == 0); ut_ad(UT_LIST_GET_LEN(trx->trx_locks) == 0);