List:Commits« Previous MessageNext Message »
From:marko.makela Date:May 14 2010 1:31pm
Subject:bzr commit into mysql-trunk-innodb branch (marko.makela:3096)
View as plain text  
#At file:///home/marko/innobase/dev/mysql/5.5-innodb/ based on revid:vasil.dimov@stripped2miapcwmz

 3096 Marko Mäkelä	2010-05-14
      Merge from mysql-5.1-innodb:
      
      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@stripped
      committer: Marko Mäkelä <marko.makela@stripped>
      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.

    added:
      mysql-test/suite/innodb/r/innodb_bug48024.result
      mysql-test/suite/innodb/t/innodb_bug48024.test
    modified:
      storage/innobase/dict/dict0dict.c
      storage/innobase/handler/ha_innodb.cc
      storage/innobase/handler/ha_innodb.h
      storage/innobase/include/dict0dict.h
      storage/innobase/include/ha_prototypes.h
      storage/innobase/include/row0mysql.h
      storage/innobase/include/trx0trx.h
      storage/innobase/row/row0mysql.c
      storage/innobase/trx/trx0i_s.c
      storage/innobase/trx/trx0trx.c
=== added file 'mysql-test/suite/innodb/r/innodb_bug48024.result'
--- a/mysql-test/suite/innodb/r/innodb_bug48024.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/innodb/r/innodb_bug48024.result	revid:marko.makela@stripped
@@ -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;

=== added file 'mysql-test/suite/innodb/t/innodb_bug48024.test'
--- a/mysql-test/suite/innodb/t/innodb_bug48024.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/innodb/t/innodb_bug48024.test	revid:marko.makela@strippedb89tea4x4uu
@@ -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;

=== modified file 'storage/innobase/dict/dict0dict.c'
--- a/storage/innobase/dict/dict0dict.c	revid:vasil.dimov@strippedmiapcwmz
+++ b/storage/innobase/dict/dict0dict.c	revid:marko.makela@stripped
@@ -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)));

=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- a/storage/innobase/handler/ha_innodb.cc	revid:vasil.dimov@stripped3075309-cqb3hwj2miapcwmz
+++ b/storage/innobase/handler/ha_innodb.cc	revid:marko.makela@strippedfe0l0b89tea4x4uu
@@ -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

=== modified file 'storage/innobase/handler/ha_innodb.h'
--- a/storage/innobase/handler/ha_innodb.h	revid:vasil.dimov@stripped309-cqb3hwj2miapcwmz
+++ b/storage/innobase/handler/ha_innodb.h	revid:marko.makela@strippedb89tea4x4uu
@@ -231,7 +231,7 @@ the definitions are bracketed with #ifde
 
 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

=== modified file 'storage/innobase/include/dict0dict.h'
--- a/storage/innobase/include/dict0dict.h	revid:vasil.dimov@stripped3hwj2miapcwmz
+++ b/storage/innobase/include/dict0dict.h	revid:marko.makela@strippedx4uu
@@ -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 */

=== modified file 'storage/innobase/include/ha_prototypes.h'
--- a/storage/innobase/include/ha_prototypes.h	revid:vasil.dimov@oracle.com-20100513075309-cqb3hwj2miapcwmz
+++ b/storage/innobase/include/ha_prototypes.h	revid:marko.makela@oracle.com-20100514133144-fe0l0b89tea4x4uu
@@ -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

=== modified file 'storage/innobase/include/row0mysql.h'
--- a/storage/innobase/include/row0mysql.h	revid:vasil.dimov@stripped100513075309-cqb3hwj2miapcwmz
+++ b/storage/innobase/include/row0mysql.h	revid:marko.makela@stripped144-fe0l0b89tea4x4uu
@@ -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 */

=== modified file 'storage/innobase/include/trx0trx.h'
--- a/storage/innobase/include/trx0trx.h	revid:vasil.dimov@stripped5309-cqb3hwj2miapcwmz
+++ b/storage/innobase/include/trx0trx.h	revid:marko.makela@stripped89tea4x4uu
@@ -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

=== modified file 'storage/innobase/row/row0mysql.c'
--- a/storage/innobase/row/row0mysql.c	revid:vasil.dimov@stripped5309-cqb3hwj2miapcwmz
+++ b/storage/innobase/row/row0mysql.c	revid:marko.makela@strippedtea4x4uu
@@ -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);

=== modified file 'storage/innobase/trx/trx0i_s.c'
--- a/storage/innobase/trx/trx0i_s.c	revid:vasil.dimov@stripped0513075309-cqb3hwj2miapcwmz
+++ b/storage/innobase/trx/trx0i_s.c	revid:marko.makela@stripped0b89tea4x4uu
@@ -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);
+
+	if (stmt != NULL) {
+
+		char	query[TRX_I_S_TRX_QUERY_MAX_LEN + 1];
 
-		TRX_I_S_STRING_COPY(*trx->mysql_query_str, row->trx_query,
-				    TRX_I_S_TRX_QUERY_MAX_LEN, cache);
+		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;
+
+	if (s != NULL && s[0] != '\0') {
 
-		TRX_I_S_STRING_COPY(trx->detailed_error,
+		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;

=== modified file 'storage/innobase/trx/trx0trx.c'
--- a/storage/innobase/trx/trx0trx.c	revid:vasil.dimov@stripped
+++ b/storage/innobase/trx/trx0trx.c	revid:marko.makela@oracle.com-20100514133144-fe0l0b89tea4x4uu
@@ -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);

Attachment: [text/bzr-bundle] bzr/marko.makela@oracle.com-20100514133144-fe0l0b89tea4x4uu.bundle
Thread
bzr commit into mysql-trunk-innodb branch (marko.makela:3096) marko.makela14 May