From 3fa0dd23e0b6df54f8c94085963264c17c3cd715 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 3 Nov 2005 14:20:13 +0300 Subject: [PATCH] A fix and a test case for Bug#14210 "Simple query with > operator on large table gives server crash": make sure that when a MyISAM temporary table is created for a cursor, it's created in its memory root, not the memory root of the current query. mysql-test/r/sp.result: Test results fixed: a test case for Bug#14210 mysql-test/t/sp.test: A test case for Bug#14210 "Simple query with > operator on large table gives server crash" sql/handler.cc: - rewrite get_new_handler to accept a memory root and use it for sql/handler.h: - get_new_handler declaration changed sql/opt_range.cc: - get_new_handler declaration changed sql/sql_base.cc: - get_new_handler declaration changed sql/sql_select.cc: - the actual fix for Bug#14210. In create_myisam_from_heap we should create the new table handler in TABLE::mem_root, not in THD::mem_root: the latter is freed shortly after cursor is open. - adjust create_tmp_table to explicitly supply &table->mem_root to get_new_handler when creating a handler for a new temporary table sql/sql_table.cc: - get_new_handler declaration changed sql/table.cc: - get_new_handler declaration changed sql/unireg.cc: - get_new_handler declaration changed tests/mysql_client_test.c: A test case for Bug#14210 "Simple query with > operator on large table gives server crash": a C API test case is worth adding because of different memory allocation/freeing patterns in handling of C API and SP cursors --- mysql-test/r/sp.result | 42 ++++++++++++++++++++++++++++++ mysql-test/t/sp.test | 52 +++++++++++++++++++++++++++++++++++++ sql/handler.cc | 36 ++++++++++++++------------ sql/handler.h | 2 +- sql/opt_range.cc | 2 +- sql/sql_base.cc | 2 +- sql/sql_select.cc | 9 ++++--- sql/sql_table.cc | 6 +++-- sql/table.cc | 3 ++- sql/unireg.cc | 4 +-- tests/mysql_client_test.c | 54 +++++++++++++++++++++++++++++++++++++++ 11 files changed, 184 insertions(+), 28 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 1f3f7dba7da..d50e6dd3751 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -3575,4 +3575,46 @@ DROP VIEW bug13095_v1 DROP PROCEDURE IF EXISTS bug13095; DROP VIEW IF EXISTS bug13095_v1; DROP TABLE IF EXISTS bug13095_t1; +drop procedure if exists bug14210| +set @@session.max_heap_table_size=16384| +select @@session.max_heap_table_size| +@@session.max_heap_table_size +16384 +create table t3 (a char(255)) engine=InnoDB| +create procedure bug14210_fill_table() +begin +declare table_size, max_table_size int default 0; +select @@session.max_heap_table_size into max_table_size; +delete from t3; +insert into t3 (a) values (repeat('a', 255)); +repeat +insert into t3 select a from t3; +select count(*)*255 from t3 into table_size; +until table_size > max_table_size*2 end repeat; +end| +call bug14210_fill_table()| +drop procedure bug14210_fill_table| +create table t4 like t3| +create procedure bug14210() +begin +declare a char(255); +declare done int default 0; +declare c cursor for select * from t3; +declare continue handler for sqlstate '02000' set done = 1; +open c; +repeat +fetch c into a; +if not done then +insert into t4 values (upper(a)); +end if; +until done end repeat; +close c; +end| +call bug14210()| +select count(*) from t4| +count(*) +256 +drop table t3, t4| +drop procedure bug14210| +set @@session.max_heap_table_size=default| drop table t1,t2; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index ab57139bb77..eaf69c0ab03 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -4488,6 +4488,58 @@ DROP TABLE IF EXISTS bug13095_t1; delimiter |; +# +# BUG#14210: "Simple query with > operator on large table gives server +# crash" +# Check that cursors work in case when HEAP tables are converted to +# MyISAM +# +--disable_warnings +drop procedure if exists bug14210| +--enable_warnings +set @@session.max_heap_table_size=16384| +select @@session.max_heap_table_size| +# To trigger the memory corruption the original table must be InnoDB. +# No harm if it's not, so don't warn if the suite is run with --skip-innodb +--disable_warnings +create table t3 (a char(255)) engine=InnoDB| +--enable_warnings +create procedure bug14210_fill_table() +begin + declare table_size, max_table_size int default 0; + select @@session.max_heap_table_size into max_table_size; + delete from t3; + insert into t3 (a) values (repeat('a', 255)); + repeat + insert into t3 select a from t3; + select count(*)*255 from t3 into table_size; + until table_size > max_table_size*2 end repeat; +end| +call bug14210_fill_table()| +drop procedure bug14210_fill_table| +create table t4 like t3| + +create procedure bug14210() +begin + declare a char(255); + declare done int default 0; + declare c cursor for select * from t3; + declare continue handler for sqlstate '02000' set done = 1; + open c; + repeat + fetch c into a; + if not done then + insert into t4 values (upper(a)); + end if; + until done end repeat; + close c; +end| +call bug14210()| +select count(*) from t4| + +drop table t3, t4| +drop procedure bug14210| +set @@session.max_heap_table_size=default| # # BUG#NNNN: New bug synopsis diff --git a/sql/handler.cc b/sql/handler.cc index 81e94af5dc7..fe0af04cf48 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -293,61 +293,61 @@ enum db_type ha_checktype(THD *thd, enum db_type database_type, } /* ha_checktype */ -handler *get_new_handler(TABLE *table, enum db_type db_type) +handler *get_new_handler(TABLE *table, MEM_ROOT *alloc, enum db_type db_type) { switch (db_type) { #ifndef NO_HASH case DB_TYPE_HASH: - return new ha_hash(table); + return new (alloc) ha_hash(table); #endif case DB_TYPE_MRG_ISAM: - return new ha_myisammrg(table); + return new (alloc) ha_myisammrg(table); #ifdef HAVE_BERKELEY_DB case DB_TYPE_BERKELEY_DB: - return new ha_berkeley(table); + return new (alloc) ha_berkeley(table); #endif #ifdef HAVE_INNOBASE_DB case DB_TYPE_INNODB: - return new ha_innobase(table); + return new (alloc) ha_innobase(table); #endif #ifdef HAVE_EXAMPLE_DB case DB_TYPE_EXAMPLE_DB: - return new ha_example(table); + return new (alloc) ha_example(table); #endif #ifdef HAVE_ARCHIVE_DB case DB_TYPE_ARCHIVE_DB: - return new ha_archive(table); + return new (alloc) ha_archive(table); #endif #ifdef HAVE_BLACKHOLE_DB case DB_TYPE_BLACKHOLE_DB: - return new ha_blackhole(table); + return new (alloc) ha_blackhole(table); #endif #ifdef HAVE_FEDERATED_DB case DB_TYPE_FEDERATED_DB: - return new ha_federated(table); + return new (alloc) ha_federated(table); #endif #ifdef HAVE_CSV_DB case DB_TYPE_CSV_DB: - return new ha_tina(table); + return new (alloc) ha_tina(table); #endif #ifdef HAVE_NDBCLUSTER_DB case DB_TYPE_NDBCLUSTER: - return new ha_ndbcluster(table); + return new (alloc) ha_ndbcluster(table); #endif case DB_TYPE_HEAP: - return new ha_heap(table); + return new (alloc) ha_heap(table); default: // should never happen { enum db_type def=(enum db_type) current_thd->variables.table_type; /* Try first with 'default table type' */ if (db_type != def) - return get_new_handler(table, def); + return get_new_handler(table, alloc, def); } /* Fall back to MyISAM */ case DB_TYPE_MYISAM: - return new ha_myisam(table); + return new (alloc) ha_myisam(table); case DB_TYPE_MRG_MYISAM: - return new ha_myisammrg(table); + return new (alloc) ha_myisammrg(table); } } @@ -1300,7 +1300,7 @@ int ha_delete_table(THD *thd, enum db_type table_type, const char *path, /* DB_TYPE_UNKNOWN is used in ALTER TABLE when renaming only .frm files */ if (table_type == DB_TYPE_UNKNOWN || - ! (file=get_new_handler(&dummy_table, table_type))) + ! (file=get_new_handler(&dummy_table, thd->mem_root, table_type))) DBUG_RETURN(ENOENT); if (lower_case_table_names == 2 && !(file->table_flags() & HA_FILE_BASED)) @@ -2469,6 +2469,7 @@ int handler::index_read_idx(byte * buf, uint index, const byte * key, TYPELIB *ha_known_exts(void) { + MEM_ROOT *mem_root= current_thd->mem_root; if (!known_extensions.type_names || mysys_usage_id != known_extensions_id) { handlerton **types; @@ -2483,7 +2484,8 @@ TYPELIB *ha_known_exts(void) { if ((*types)->state == SHOW_OPTION_YES) { - handler *file= get_new_handler(0,(enum db_type) (*types)->db_type); + handler *file= get_new_handler(0, mem_root, + (enum db_type) (*types)->db_type); for (ext= file->bas_ext(); *ext; ext++) { while ((old_ext= it++)) diff --git a/sql/handler.h b/sql/handler.h index af80f021e75..e16c428954c 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -874,7 +874,7 @@ extern ulong total_ha, total_ha_2pc; /* lookups */ enum db_type ha_resolve_by_name(const char *name, uint namelen); const char *ha_get_storage_engine(enum db_type db_type); -handler *get_new_handler(TABLE *table, enum db_type db_type); +handler *get_new_handler(TABLE *table, MEM_ROOT *alloc, enum db_type db_type); enum db_type ha_checktype(THD *thd, enum db_type database_type, bool no_substitute, bool report_error); bool ha_check_storage_engine_flag(enum db_type db_type, uint32 flag); diff --git a/sql/opt_range.cc b/sql/opt_range.cc index ff2b14a27ee..1dc2996e351 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -931,7 +931,7 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler) } THD *thd= current_thd; - if (!(file= get_new_handler(head, head->s->db_type))) + if (!(file= get_new_handler(head, thd->mem_root, head->s->db_type))) goto failure; DBUG_PRINT("info", ("Allocated new handler %p", file)); if (file->ha_open(head->s->path, head->db_stat, HA_OPEN_IGNORE_IF_LOCKED)) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 5c929dcdf69..15fb477b0d1 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2634,7 +2634,7 @@ bool rm_temporary_table(enum db_type base, char *path) if (my_delete(path,MYF(0))) error=1; /* purecov: inspected */ *fn_ext(path)='\0'; // remove extension - handler *file=get_new_handler((TABLE*) 0, base); + handler *file= get_new_handler((TABLE*) 0, current_thd->mem_root, base); if (file && file->delete_table(path)) { error=1; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 285a5c5696b..f6e1f7e80e9 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -8279,7 +8279,8 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List &fields, (select_options & (OPTION_BIG_TABLES | SELECT_SMALL_RESULT)) == OPTION_BIG_TABLES || (select_options & TMP_TABLE_FORCE_MYISAM)) { - table->file=get_new_handler(table,table->s->db_type= DB_TYPE_MYISAM); + table->file= get_new_handler(table, &table->mem_root, + table->s->db_type= DB_TYPE_MYISAM); if (group && (param->group_parts > table->file->max_key_parts() || param->group_length > table->file->max_key_length())) @@ -8287,7 +8288,8 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List &fields, } else { - table->file=get_new_handler(table,table->s->db_type= DB_TYPE_HEAP); + table->file= get_new_handler(table, &table->mem_root, + table->s->db_type= DB_TYPE_HEAP); } if (!using_unique_constraint) @@ -8871,7 +8873,8 @@ bool create_myisam_from_heap(THD *thd, TABLE *table, TMP_TABLE_PARAM *param, new_table= *table; new_table.s= &new_table.share_not_to_be_used; new_table.s->db_type= DB_TYPE_MYISAM; - if (!(new_table.file= get_new_handler(&new_table,DB_TYPE_MYISAM))) + if (!(new_table.file= get_new_handler(&new_table, &new_table.mem_root, + DB_TYPE_MYISAM))) DBUG_RETURN(1); // End of memory save_proc_info=thd->proc_info; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index b635c44c6dc..ee5acea87c0 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1508,7 +1508,7 @@ bool mysql_create_table(THD *thd,const char *db, const char *table_name, if (create_info->row_type == ROW_TYPE_DYNAMIC) db_options|=HA_OPTION_PACK_RECORD; alias= table_case_name(create_info, table_name); - file=get_new_handler((TABLE*) 0, create_info->db_type); + file= get_new_handler((TABLE*) 0, thd->mem_root, create_info->db_type); #ifdef NOT_USED /* @@ -1806,10 +1806,12 @@ mysql_rename_table(enum db_type base, const char *new_db, const char *new_name) { + THD *thd= current_thd; char from[FN_REFLEN], to[FN_REFLEN], lc_from[FN_REFLEN], lc_to[FN_REFLEN]; char *from_base= from, *to_base= to; char tmp_name[NAME_LEN+1]; - handler *file=(base == DB_TYPE_UNKNOWN ? 0 : get_new_handler((TABLE*) 0, base)); + handler *file= (base == DB_TYPE_UNKNOWN ? 0 : + get_new_handler((TABLE*) 0, thd->mem_root, base)); int error=0; DBUG_ENTER("mysql_rename_table"); diff --git a/sql/table.cc b/sql/table.cc index dd6018b70f3..1acf49bc03e 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -338,7 +338,8 @@ int openfrm(THD *thd, const char *name, const char *alias, uint db_stat, my_free(buff, MYF(0)); } /* Allocate handler */ - if (!(outparam->file= get_new_handler(outparam, share->db_type))) + if (!(outparam->file= get_new_handler(outparam, &outparam->mem_root, + share->db_type))) goto err; error=4; diff --git a/sql/unireg.cc b/sql/unireg.cc index 065f8583f71..0ab77462f61 100644 --- a/sql/unireg.cc +++ b/sql/unireg.cc @@ -90,7 +90,7 @@ bool mysql_create_frm(THD *thd, my_string file_name, if (!(screen_buff=pack_screens(create_fields,&info_length,&screens,0))) DBUG_RETURN(1); if (db_file == NULL) - db_file= get_new_handler((TABLE*) 0, create_info->db_type); + db_file= get_new_handler((TABLE*) 0, thd->mem_root, create_info->db_type); /* If fixed row records, we need one bit to check for deleted rows */ if (!(create_info->table_options & HA_OPTION_PACK_RECORD)) @@ -699,7 +699,7 @@ static bool make_empty_rec(THD *thd, File file,enum db_type table_type, /* We need a table to generate columns for default values */ bzero((char*) &table,sizeof(table)); table.s= &table.share_not_to_be_used; - handler= get_new_handler((TABLE*) 0, table_type); + handler= get_new_handler((TABLE*) 0, thd->mem_root, table_type); if (!handler || !(buff=(uchar*) my_malloc((uint) reclength,MYF(MY_WME | MY_ZEROFILL)))) diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index f46b1cf1784..949c3d5bf1c 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -14319,6 +14319,7 @@ static void test_bug12243() mysql_autocommit(mysql, TRUE); /* restore default */ } + /* Bug#11718: query with function, join and order by returns wrong type */ @@ -14366,6 +14367,58 @@ static void test_bug12925() } +/* + Bug#14210 "Simple query with > operator on large table gives server + crash" +*/ + +static void test_bug14210() +{ + MYSQL_STMT *stmt; + int rc, i; + const char *stmt_text; + ulong type; + + myheader("test_bug14210"); + + mysql_query(mysql, "drop table if exists t1"); + /* + To trigger the problem the table must be InnoDB, although the problem + itself is not InnoDB related. In case the table is MyISAM this test + is harmless. + */ + mysql_query(mysql, "create table t1 (a varchar(255)) type=InnoDB"); + rc= mysql_query(mysql, "insert into t1 (a) values (repeat('a', 256))"); + myquery(rc); + rc= mysql_query(mysql, "set @@session.max_heap_table_size=16384"); + /* Create a big enough table (more than max_heap_table_size) */ + for (i= 0; i < 8; i++) + { + rc= mysql_query(mysql, "insert into t1 (a) select a from t1"); + myquery(rc); + } + /* create statement */ + stmt= mysql_stmt_init(mysql); + type= (ulong) CURSOR_TYPE_READ_ONLY; + mysql_stmt_attr_set(stmt, STMT_ATTR_CURSOR_TYPE, (const void*) &type); + + stmt_text= "select a from t1"; + + rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text)); + check_execute(stmt, rc); + rc= mysql_stmt_execute(stmt); + while ((rc= mysql_stmt_fetch(stmt)) == 0) + ; + DIE_UNLESS(rc == MYSQL_NO_DATA); + + rc= mysql_stmt_close(stmt); + + rc= mysql_query(mysql, "drop table t1"); + myquery(rc); + rc= mysql_query(mysql, "set @@session.max_heap_table_size=default"); + myquery(rc); +} + /* Read and parse arguments and MySQL options from my.cnf */ @@ -14621,6 +14674,7 @@ static struct my_tests_st my_tests[]= { { "test_bug11901", test_bug11901 }, { "test_bug11904", test_bug11904 }, { "test_bug12243", test_bug12243 }, + { "test_bug14210", test_bug14210 }, { 0, 0 } };