← Back to team overview

maria-developers team mailing list archive

bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (knielsen:2714)

 

#At lp:maria

 2714 knielsen@xxxxxxxxxxxxxxx	2009-06-18
      Fix test cases after merge of XtraDB into MariaDB.
      Manually merge some InnoDB changes into XtraDB.
      Fix ALTER TABLE bug in XtraDB with wrong comparison of row type.
      modified:
        mysql-test/include/varchar.inc
        mysql-test/mysql-test-run.pl
        mysql-test/r/innodb.result
        mysql-test/t/innodb-use-sys-malloc.test
        sql/sql_table.cc
        storage/xtradb/handler/ha_innodb.cc
        storage/xtradb/include/pars0pars.h
        storage/xtradb/include/univ.i

per-file messages:
  mysql-test/include/varchar.inc
    Fix in test case that which of several duplicate keys triggers an error is not
    deterministic.
  mysql-test/mysql-test-run.pl
    InnoDB does not bother to free resources individually during shutdown, but due to using
    its own memory tracking it nevertheless can free everything at exit. But XtraDB adds an
    option, on by default, to skip this extra tracking. This causes lots of Valgrind
    warnings, so needs to be disabled for Valgrind testing.
  mysql-test/r/innodb.result
    Fix in test case that which of several duplicate keys triggers an error is not
    deterministic.
  mysql-test/t/innodb-use-sys-malloc.test
    InnoDB does not bother to free resources individually during shutdown, but due to using
    its own memory tracking it nevertheless can free everything at exit. But XtraDB adds an
    option, on by default, to skip this extra tracking. This causes lots of Valgrind
    warnings, so needs to be disabled for Valgrind testing.
  sql/sql_table.cc
    Add some useful DBUG while debugging alter table.
  storage/xtradb/handler/ha_innodb.cc
    Fix that check_if_incompatible_data did not realise that ROW_TYPE_DEFAULT is identical
    to the default row format ROW_TYPE_COMPACT, causing excessive table copying in
    ALTER TABLE
    Add some useful DBUG while debugging alter table.
    Manually merge into XtraDB a few small changes for InnoDB from upstream MySQL.
  storage/xtradb/include/pars0pars.h
    Manually merge into XtraDB a few small changes for InnoDB from upstream MySQL.
  storage/xtradb/include/univ.i
    Manually merge a MariaDB fix in InnoDB into XtraDB.
=== modified file 'mysql-test/include/varchar.inc'
--- a/mysql-test/include/varchar.inc	2008-05-14 06:50:16 +0000
+++ b/mysql-test/include/varchar.inc	2009-06-18 12:39:21 +0000
@@ -86,6 +86,8 @@ explain select count(*) from t1 where v 
 --replace_column 9 #
 explain select count(*) from t1 where v between 'a' and 'a ' and v between 'a  ' and 'b\n';
 
+# Which duplicate entry triggers error is not deterministic.
+--replace_regex /Duplicate entry '[^']+' for key/Duplicate entry '{ ' for key/
 --error ER_DUP_ENTRY
 alter table t1 add unique(v);
 alter table t1 add key(v);

=== modified file 'mysql-test/mysql-test-run.pl'
--- a/mysql-test/mysql-test-run.pl	2009-06-05 15:35:22 +0000
+++ b/mysql-test/mysql-test-run.pl	2009-06-18 12:39:21 +0000
@@ -1356,6 +1356,18 @@ sub command_line_setup {
 	       join(" ", @valgrind_args), "\"");
   }
 
+  # InnoDB does not bother to do individual de-allocations at exit. Instead it
+  # relies on a custom allocator to track every allocation, and frees all at
+  # once during exit.
+  # In XtraDB, an option use-sys-malloc is introduced (and on by default) to
+  # disable this (for performance). But this exposes Valgrind to all the
+  # missing de-allocations, so we need to disable it to at least get
+  # meaningful leak checking for the rest of the server.
+  if ($opt_valgrind_mysqld)
+  {
+    push(@opt_extra_mysqld_opt, "--loose-skip-innodb-use-sys-malloc");
+  }
+
   mtr_report("Checking supported features...");
 
   check_ndbcluster_support(\%mysqld_variables);

=== modified file 'mysql-test/r/innodb.result'
--- a/mysql-test/r/innodb.result	2009-06-09 15:08:46 +0000
+++ b/mysql-test/r/innodb.result	2009-06-18 12:39:21 +0000
@@ -1970,7 +1970,7 @@ explain select count(*) from t1 where v 
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	ref	v	v	13	const	#	Using where; Using index
 alter table t1 add unique(v);
-ERROR 23000: Duplicate entry 'v' for key 'v_2'
+ERROR 23000: Duplicate entry '{ ' for key 'v_2'
 alter table t1 add key(v);
 select concat('*',v,'*',c,'*',t,'*') as qq from t1 where v='a';
 qq

=== modified file 'mysql-test/t/innodb-use-sys-malloc.test'
--- a/mysql-test/t/innodb-use-sys-malloc.test	2009-06-09 13:19:13 +0000
+++ b/mysql-test/t/innodb-use-sys-malloc.test	2009-06-18 12:39:21 +0000
@@ -1,4 +1,7 @@
 --source include/have_innodb.inc
+# XtraDB has lots of memory leak warnings at shutdown when
+# --innodb-use-sys-malloc
+--source include/not_valgrind.inc
 
 #display current value of innodb_use_sys_malloc
 SELECT @@GLOBAL.innodb_use_sys_malloc;

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2009-06-02 09:58:27 +0000
+++ b/sql/sql_table.cc	2009-06-18 12:39:21 +0000
@@ -5333,6 +5333,7 @@ compare_tables(TABLE *table,
       !table->s->mysql_version ||
       (table->s->frm_version < FRM_VER_TRUE_VARCHAR && varchar))
   {
+    DBUG_PRINT("info", ("Basic checks -> ALTER_TABLE_DATA_CHANGED"));
     *need_copy_table= ALTER_TABLE_DATA_CHANGED;
     DBUG_RETURN(0);
   }
@@ -5361,6 +5362,8 @@ compare_tables(TABLE *table,
     if ((tmp_new_field->flags & NOT_NULL_FLAG) !=
 	(uint) (field->flags & NOT_NULL_FLAG))
     {
+      DBUG_PRINT("info", ("NULL behaviour difference in field '%s' -> "
+                          "ALTER_TABLE_DATA_CHANGED", new_field->field_name));
       *need_copy_table= ALTER_TABLE_DATA_CHANGED;
       DBUG_RETURN(0);
     }
@@ -5382,6 +5385,8 @@ compare_tables(TABLE *table,
     /* Evaluate changes bitmap and send to check_if_incompatible_data() */
     if (!(tmp= field->is_equal(tmp_new_field)))
     {
+      DBUG_PRINT("info", ("!field_is_equal('%s') -> ALTER_TABLE_DATA_CHANGED",
+                          new_field->field_name));
       *need_copy_table= ALTER_TABLE_DATA_CHANGED;
       DBUG_RETURN(0);
     }
@@ -5515,16 +5520,22 @@ compare_tables(TABLE *table,
   /* Check if changes are compatible with current handler without a copy */
   if (table->file->check_if_incompatible_data(create_info, changes))
   {
+    DBUG_PRINT("info", ("check_if_incompatible_data() -> "
+                        "ALTER_TABLE_DATA_CHANGED"));
     *need_copy_table= ALTER_TABLE_DATA_CHANGED;
     DBUG_RETURN(0);
   }
 
   if (*index_drop_count || *index_add_count)
   {
+    DBUG_PRINT("info", ("Index dropped=%u added=%u -> "
+                        "ALTER_TABLE_INDEX_CHANGED",
+                        *index_drop_count, *index_add_count));
     *need_copy_table= ALTER_TABLE_INDEX_CHANGED;
     DBUG_RETURN(0);
   }
 
+  DBUG_PRINT("info", (" -> ALTER_TABLE_METADATA_ONLY"));
   *need_copy_table= ALTER_TABLE_METADATA_ONLY; // Tables are compatible
   DBUG_RETURN(0);
 }

=== modified file 'storage/xtradb/handler/ha_innodb.cc'
--- a/storage/xtradb/handler/ha_innodb.cc	2009-06-11 12:53:26 +0000
+++ b/storage/xtradb/handler/ha_innodb.cc	2009-06-18 12:39:21 +0000
@@ -5018,7 +5018,6 @@ convert_search_mode_to_innobase(
 	case HA_READ_MBR_WITHIN:
 	case HA_READ_MBR_DISJOINT:
 	case HA_READ_MBR_EQUAL:
-		my_error(ER_TABLE_CANT_HANDLE_SPKEYS, MYF(0));
 		return(PAGE_CUR_UNSUPP);
 	/* do not use "default:" in order to produce a gcc warning:
 	enumeration value '...' not handled in switch
@@ -6720,6 +6719,7 @@ innobase_rename_table(
 	int	error;
 	char*	norm_to;
 	char*	norm_from;
+	DBUG_ENTER("innobase_rename_table");
 
 	if (lower_case_table_names) {
 		srv_lower_case_table_names = TRUE;
@@ -6747,6 +6747,7 @@ innobase_rename_table(
 	if (error != DB_SUCCESS) {
 		FILE* ef = dict_foreign_err_file;
 
+		DBUG_PRINT("info", ("rename failed: %d", error));
 		fputs("InnoDB: Renaming table ", ef);
 		ut_print_name(ef, trx, TRUE, norm_from);
 		fputs(" to ", ef);
@@ -6767,7 +6768,7 @@ innobase_rename_table(
 	my_free(norm_to, MYF(0));
 	my_free(norm_from, MYF(0));
 
-	return error;
+	DBUG_RETURN(error);
 }
 /*************************************************************************
 Renames an InnoDB table. */
@@ -6900,7 +6901,7 @@ ha_innobase::records_in_range(
 						      mode2);
 	} else {
 
-		n_rows = 0;
+		n_rows = HA_POS_ERROR;
 	}
 
 	mem_heap_free(heap);
@@ -7614,7 +7615,7 @@ ha_innobase::get_foreign_key_list(THD *t
 	    f_key_info.referenced_key_name = thd_make_lex_string(
 		    thd, f_key_info.referenced_key_name,
 		    foreign->referenced_index->name,
-		    strlen(foreign->referenced_index->name), 1);
+		    (uint) strlen(foreign->referenced_index->name), 1);
           }
           else
             f_key_info.referenced_key_name= 0;
@@ -8227,7 +8228,7 @@ innodb_show_status(
 
 	bool result = FALSE;
 
-	if (stat_print(thd, innobase_hton_name, strlen(innobase_hton_name),
+	if (stat_print(thd, innobase_hton_name, (uint) strlen(innobase_hton_name),
 			STRING_WITH_LEN(""), str, flen)) {
 		result= TRUE;
 	}
@@ -8258,7 +8259,7 @@ innodb_mutex_show_status(
 	ulint	  rw_lock_count_os_yield= 0;
 	ulonglong rw_lock_wait_time= 0;
 #endif /* UNIV_DEBUG */
-	uint	  hton_name_len= strlen(innobase_hton_name), buf1len, buf2len;
+	uint	  hton_name_len= (uint) strlen(innobase_hton_name), buf1len, buf2len;
 	DBUG_ENTER("innodb_mutex_show_status");
 	DBUG_ASSERT(hton == innodb_hton_ptr);
 
@@ -8302,9 +8303,9 @@ innodb_mutex_show_status(
 			rw_lock_wait_time += mutex->lspent_time;
 		}
 #else /* UNIV_DEBUG */
-		buf1len= my_snprintf(buf1, sizeof(buf1), "%s:%lu",
+		buf1len= (uint) my_snprintf(buf1, sizeof(buf1), "%s:%lu",
 				     mutex->cfile_name, (ulong) mutex->cline);
-		buf2len= my_snprintf(buf2, sizeof(buf2), "os_waits=%lu",
+		buf2len= (uint) my_snprintf(buf2, sizeof(buf2), "os_waits=%lu",
 				     mutex->count_os_wait);
 
 		if (stat_print(thd, innobase_hton_name,
@@ -8860,7 +8861,7 @@ ha_innobase::get_error_message(int error
 {
 	trx_t*	trx = check_trx_exists(ha_thd());
 
-	buf->copy(trx->detailed_error, strlen(trx->detailed_error),
+	buf->copy(trx->detailed_error, (uint) strlen(trx->detailed_error),
 		system_charset_info);
 
 	return(FALSE);
@@ -9294,31 +9295,49 @@ ha_innobase::check_if_incompatible_data(
 	HA_CREATE_INFO*	info,
 	uint		table_changes)
 {
+	enum row_type row_type, info_row_type;
+	DBUG_ENTER("ha_innobase::check_if_incompatible_data");
+
 	if (table_changes != IS_EQUAL_YES) {
 
-		return(COMPATIBLE_DATA_NO);
+		DBUG_PRINT("info", ("table_changes != IS_EQUAL_YES "
+				    "-> COMPATIBLE_DATA_NO"));
+		DBUG_RETURN(COMPATIBLE_DATA_NO);
 	}
 
 	/* Check that auto_increment value was not changed */
 	if ((info->used_fields & HA_CREATE_USED_AUTO) &&
 		info->auto_increment_value != 0) {
 
-		return(COMPATIBLE_DATA_NO);
+		DBUG_PRINT("info", ("auto_increment_value changed -> "
+				    "COMPATIBLE_DATA_NO"));
+		DBUG_RETURN(COMPATIBLE_DATA_NO);
 	}
 
 	/* Check that row format didn't change */
+	row_type = get_row_type();
+	info_row_type = info->row_type;
+	/* Default is compact. */
+	if (info_row_type == ROW_TYPE_DEFAULT)
+		info_row_type = ROW_TYPE_COMPACT;
 	if ((info->used_fields & HA_CREATE_USED_ROW_FORMAT) &&
-	    get_row_type() != info->row_type) {
+	    row_type != info_row_type) {
 
-		return(COMPATIBLE_DATA_NO);
+		DBUG_PRINT("info", ("get_row_type()=%d != info->row_type=%d -> "
+				    "COMPATIBLE_DATA_NO",
+				    row_type, info->row_type));
+		DBUG_RETURN(COMPATIBLE_DATA_NO);
 	}
 
 	/* Specifying KEY_BLOCK_SIZE requests a rebuild of the table. */
 	if (info->used_fields & HA_CREATE_USED_KEY_BLOCK_SIZE) {
-		return(COMPATIBLE_DATA_NO);
+		DBUG_PRINT("info", ("HA_CREATE_USED_KEY_BLOCK_SIZE -> "
+				    "COMPATIBLE_DATA_NO"));
+		DBUG_RETURN(COMPATIBLE_DATA_NO);
 	}
 
-	return(COMPATIBLE_DATA_YES);
+	DBUG_PRINT("info", (" -> COMPATIBLE_DATA_YES"));
+	DBUG_RETURN(COMPATIBLE_DATA_YES);
 }
 
 /****************************************************************

=== modified file 'storage/xtradb/include/pars0pars.h'
--- a/storage/xtradb/include/pars0pars.h	2009-03-26 06:11:11 +0000
+++ b/storage/xtradb/include/pars0pars.h	2009-06-18 12:39:21 +0000
@@ -700,7 +700,7 @@ struct for_node_struct{
 					definition */
 	que_node_t*	loop_start_limit;/* initial value of loop variable */
 	que_node_t*	loop_end_limit;	/* end value of loop variable */
-	int		loop_end_value;	/* evaluated value for the end value:
+	lint		loop_end_value;	/* evaluated value for the end value:
 					it is calculated only when the loop
 					is entered, and will not change within
 					the loop */

=== modified file 'storage/xtradb/include/univ.i'
--- a/storage/xtradb/include/univ.i	2009-06-11 12:53:26 +0000
+++ b/storage/xtradb/include/univ.i	2009-06-18 12:39:21 +0000
@@ -133,10 +133,10 @@ from Makefile.in->ut0auxconf.h */
 # endif /* HAVE_ATOMIC_PTHREAD_T */
 #endif /* HAVE_GCC_ATOMIC_BUILTINS */
 
-/* We only try to do explicit inlining of functions with gcc and
-Microsoft Visual C++ */
+/* Enable explicit inlining of functions only for compilers known to
+support it. */
 
-# if !defined(__GNUC__)
+# if !defined(__GNUC__) && !defined(__SUNPRO_C)
 #  undef  UNIV_MUST_NOT_INLINE			/* Remove compiler warning */
 #  define UNIV_MUST_NOT_INLINE
 # endif




Follow ups