From e67bdaf604418ce51fc74db2b69f143a28ef9746 Mon Sep 17 00:00:00 2001 From: "ingo@mysql.com" <> Date: Tue, 27 Jun 2006 11:26:41 +0200 Subject: Bug#11824 - internal /tmp/*.{MYD,MYI} files remain, causing subsequent queries to fail Very complex select statements can create temporary tables that are too big to be represented as a MyISAM table. This was not checked at table creation time, but only at open time. The result was an attempt to delete the "impossible" table. But if the server is built --with-raid, MyISAM tries to open the table before deleting the files. It needs to find out if the table uses the raid support and how many raid chunks there are. This is done with an open "for repair", which will almost always succeed. But in this case we have an "impossible" table. The open failed. Hence the files were not deleted. Also the error message was a bit unspecific. I turned an open error in this situation into the assumption of having no raid support on the table. Thus the normal data file is tried to be deleted. This may however leave existing raid chunks behind. I also added a check in mi_create() to prevent the creation of an "impossible" table. A more decriptive error message is given in this case. No test case. The required select statement is way too large for the test suite. I added a test script to the bug report. --- myisam/mi_create.c | 17 +++++++++++++++++ myisam/mi_delete_table.c | 24 ++++++++++++++++++------ 2 files changed, 35 insertions(+), 6 deletions(-) (limited to 'myisam') diff --git a/myisam/mi_create.c b/myisam/mi_create.c index 41c965c7c80..4183040500b 100644 --- a/myisam/mi_create.c +++ b/myisam/mi_create.c @@ -59,6 +59,8 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, my_off_t key_root[MI_MAX_POSSIBLE_KEY],key_del[MI_MAX_KEY_BLOCK_SIZE]; MI_CREATE_INFO tmp_create_info; DBUG_ENTER("mi_create"); + DBUG_PRINT("enter", ("keys: %u columns: %u uniques: %u flags: %u", + keys, columns, uniques, flags)); if (!ci) { @@ -447,6 +449,16 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, uniques * MI_UNIQUEDEF_SIZE + (key_segs + unique_key_parts)*HA_KEYSEG_SIZE+ columns*MI_COLUMNDEF_SIZE); + DBUG_PRINT("info", ("info_length: %u", info_length)); + /* There are only 16 bits for the total header length. */ + if (info_length > 65535) + { + my_printf_error(0, "MyISAM table '%s' has too many columns and/or " + "indexes and/or unique constraints.", + MYF(0), name + dirname_length(name)); + my_errno= HA_WRONG_CREATE_OPTION; + goto err; + } bmove(share.state.header.file_version,(byte*) myisam_file_magic,4); ci->old_options=options| (ci->old_options & HA_OPTION_TEMP_COMPRESS_RECORD ? @@ -594,6 +606,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, errpos=3; } + DBUG_PRINT("info", ("write state info and base info")); if (mi_state_info_write(file, &share.state, 2) || mi_base_info_write(file, &share.base)) goto err; @@ -607,6 +620,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, #endif /* Write key and keyseg definitions */ + DBUG_PRINT("info", ("write key and keyseg definitions")); for (i=0 ; i < share.base.keys - uniques; i++) { uint sp_segs=(keydefs[i].flag & HA_SPATIAL) ? 2*SPDIMS : 0; @@ -655,6 +669,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, } /* Save unique definition */ + DBUG_PRINT("info", ("write unique definitions")); for (i=0 ; i < share.state.header.uniques ; i++) { if (mi_uniquedef_write(file, &uniquedefs[i])) @@ -665,6 +680,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, goto err; } } + DBUG_PRINT("info", ("write field definitions")); for (i=0 ; i < share.base.fields ; i++) if (mi_recinfo_write(file, &recinfo[i])) goto err; @@ -679,6 +695,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, #endif /* Enlarge files */ + DBUG_PRINT("info", ("enlarge to keystart: %lu", (ulong) share.base.keystart)); if (my_chsize(file,(ulong) share.base.keystart,0,MYF(0))) goto err; diff --git a/myisam/mi_delete_table.c b/myisam/mi_delete_table.c index 6843881568d..2fba31cf8be 100644 --- a/myisam/mi_delete_table.c +++ b/myisam/mi_delete_table.c @@ -34,12 +34,24 @@ int mi_delete_table(const char *name) #ifdef USE_RAID { MI_INFO *info; - /* we use 'open_for_repair' to be able to delete a crashed table */ - if (!(info=mi_open(name, O_RDONLY, HA_OPEN_FOR_REPAIR))) - DBUG_RETURN(my_errno); - raid_type = info->s->base.raid_type; - raid_chunks = info->s->base.raid_chunks; - mi_close(info); + /* + When built with RAID support, we need to determine if this table + makes use of the raid feature. If yes, we need to remove all raid + chunks. This is done with my_raid_delete(). Unfortunately it is + necessary to open the table just to check this. We use + 'open_for_repair' to be able to open even a crashed table. If even + this open fails, we assume no raid configuration for this table + and try to remove the normal data file only. This may however + leave the raid chunks behind. + */ + if (!(info= mi_open(name, O_RDONLY, HA_OPEN_FOR_REPAIR))) + raid_type= 0; + else + { + raid_type= info->s->base.raid_type; + raid_chunks= info->s->base.raid_chunks; + mi_close(info); + } } #ifdef EXTRA_DEBUG check_table_is_closed(name,"delete"); -- cgit v1.2.1 From d8499f2d8f2ad08145c32e274a8cdd02c5933e01 Mon Sep 17 00:00:00 2001 From: "ingo@mysql.com" <> Date: Wed, 28 Jun 2006 14:27:37 +0200 Subject: Bug#17877 - Corrupted spatial index CHECK TABLE could complain about a fully intact spatial index. A wrong comparison operator was used for table checking. The result was that it checked for non-matching spatial keys. This succeeded if at least two different keys were present, but failed if only the matching key was present. I fixed the key comparison. --- myisam/mi_check.c | 5 +++-- myisam/mi_key.c | 2 +- myisam/rt_index.c | 8 +++++--- myisam/rt_mbr.c | 6 +++++- 4 files changed, 14 insertions(+), 7 deletions(-) (limited to 'myisam') diff --git a/myisam/mi_check.c b/myisam/mi_check.c index 2395640d5bf..1e62e5e641d 100644 --- a/myisam/mi_check.c +++ b/myisam/mi_check.c @@ -1155,12 +1155,13 @@ int chk_data_link(MI_CHECK *param, MI_INFO *info,int extend) */ int search_result= (keyinfo->flag & HA_SPATIAL) ? rtree_find_first(info, key, info->lastkey, key_length, - SEARCH_SAME) : + MBR_EQUAL | MBR_DATA) : _mi_search(info,keyinfo,info->lastkey,key_length, SEARCH_SAME, info->s->state.key_root[key]); if (search_result) { - mi_check_print_error(param,"Record at: %10s Can't find key for index: %2d", + mi_check_print_error(param,"Record at: %10s " + "Can't find key for index: %2d", llstr(start_recpos,llbuff),key+1); if (error++ > MAXERR || !(param->testflag & T_VERBOSE)) goto err2; diff --git a/myisam/mi_key.c b/myisam/mi_key.c index cb85febd869..eaa854b1a37 100644 --- a/myisam/mi_key.c +++ b/myisam/mi_key.c @@ -54,7 +54,7 @@ uint _mi_make_key(register MI_INFO *info, uint keynr, uchar *key, TODO: nulls processing */ #ifdef HAVE_SPATIAL - return sp_make_key(info,keynr,key,record,filepos); + DBUG_RETURN(sp_make_key(info,keynr,key,record,filepos)); #else DBUG_ASSERT(0); /* mi_open should check that this never happens*/ #endif diff --git a/myisam/rt_index.c b/myisam/rt_index.c index 97554dca4e6..1806476dc39 100644 --- a/myisam/rt_index.c +++ b/myisam/rt_index.c @@ -183,9 +183,11 @@ int rtree_find_first(MI_INFO *info, uint keynr, uchar *key, uint key_length, return -1; } - /* Save searched key */ - memcpy(info->first_mbr_key, key, keyinfo->keylength - - info->s->base.rec_reflength); + /* + Save searched key, include data pointer. + The data pointer is required if the search_flag contains MBR_DATA. + */ + memcpy(info->first_mbr_key, key, keyinfo->keylength); info->last_rkey_length = key_length; info->rtree_recursion_depth = -1; diff --git a/myisam/rt_mbr.c b/myisam/rt_mbr.c index c43daec2f7c..897862c1c9a 100644 --- a/myisam/rt_mbr.c +++ b/myisam/rt_mbr.c @@ -52,10 +52,14 @@ if (EQUAL_CMP(amin, amax, bmin, bmax)) \ return 1; \ } \ - else /* if (nextflag & MBR_DISJOINT) */ \ + else if (nextflag & MBR_DISJOINT) \ { \ if (DISJOINT_CMP(amin, amax, bmin, bmax)) \ return 1; \ + }\ + else /* if unknown comparison operator */ \ + { \ + DBUG_ASSERT(0); \ } #define RT_CMP_KORR(type, korr_func, len, nextflag) \ -- cgit v1.2.1 From 9dd5bc3843a44072cc7f211b6ad8cf3102e81cc1 Mon Sep 17 00:00:00 2001 From: "ingo@mysql.com" <> Date: Wed, 28 Jun 2006 16:07:39 +0200 Subject: Bug#19835 - Binary copy of corrupted tables crash the server when issuing a query A corrupt table with dynamic record format can crash the server when trying to select from it. I fixed the crash that resulted from the particular type of corruption that has been reported for this bug. No test case. To test it, one needs a table with a very special corruption. The bug report contains a file with such a table. --- myisam/mi_dynrec.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'myisam') diff --git a/myisam/mi_dynrec.c b/myisam/mi_dynrec.c index 43783ca2d36..1b691c955f1 100644 --- a/myisam/mi_dynrec.c +++ b/myisam/mi_dynrec.c @@ -1116,6 +1116,9 @@ int _mi_read_dynamic_record(MI_INFO *info, my_off_t filepos, byte *buf) info->rec_cache.pos_in_file <= block_info.next_filepos && flush_io_cache(&info->rec_cache)) goto err; + /* A corrupted table can have wrong pointers. (Bug# 19835) */ + if (block_info.next_filepos == HA_OFFSET_ERROR) + goto panic; info->rec_cache.seek_not_done=1; if ((b_type=_mi_get_block_info(&block_info,file, block_info.next_filepos)) -- cgit v1.2.1 From 99ad23ec7cde9db5f02ca7ac375b0050289d711f Mon Sep 17 00:00:00 2001 From: "ingo@mysql.com" <> Date: Wed, 28 Jun 2006 18:55:30 +0200 Subject: Bug#14400 - Query joins wrong rows from table which is subject of "concurrent insert" It was possible that fetching a record by an exact key value (including the record pointer) could return a record with a different key value. This happened only if a concurrent insert added a record with the searched key value after the fetching statement locked the table for read. The search succeded on the key value, but the record was rejected as it was past the file length that was remembered at start of the fetching statement. With other words it was rejected as being a concurrently inserted record. The action to recover from this problem was to fetch the record that is pointed at by the next key of the index. This was repeated until a record below the file length was found. I do now avoid this loop if an exact match was searched. If this match is beyond the file length, it is now treated as "key not found". There cannot be another key with the same record pointer. --- myisam/mi_rkey.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'myisam') diff --git a/myisam/mi_rkey.c b/myisam/mi_rkey.c index 70122288d6c..41c2e173b70 100644 --- a/myisam/mi_rkey.c +++ b/myisam/mi_rkey.c @@ -66,6 +66,7 @@ int mi_rkey(MI_INFO *info, byte *buf, int inx, const byte *key, uint key_len, if (fast_mi_readinfo(info)) goto err; + if (share->concurrent_insert) rw_rdlock(&share->key_root_lock[inx]); @@ -77,14 +78,24 @@ int mi_rkey(MI_INFO *info, byte *buf, int inx, const byte *key, uint key_len, if (!_mi_search(info,keyinfo, key_buff, use_key_length, myisam_read_vec[search_flag], info->s->state.key_root[inx])) { - while (info->lastpos >= info->state->data_file_length) + /* + If we are searching for an exact key (including the data pointer) + and this was added by an concurrent insert, + then the result is "key not found". + */ + if ((search_flag == HA_READ_KEY_EXACT) && + (info->lastpos >= info->state->data_file_length)) + { + my_errno= HA_ERR_KEY_NOT_FOUND; + info->lastpos= HA_OFFSET_ERROR; + } + else while (info->lastpos >= info->state->data_file_length) { /* Skip rows that are inserted by other threads since we got a lock Note that this can only happen if we are not searching after an exact key, because the keys are sorted according to position */ - if (_mi_search_next(info, keyinfo, info->lastkey, info->lastkey_length, myisam_readnext_vec[search_flag], @@ -92,6 +103,7 @@ int mi_rkey(MI_INFO *info, byte *buf, int inx, const byte *key, uint key_len, break; } } + if (share->concurrent_insert) rw_unlock(&share->key_root_lock[inx]); -- cgit v1.2.1