From 28131daa266e90e223c4b8847346d5dc55c5ffaf Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 11 Apr 2008 11:53:21 +0200 Subject: [PATCH] Fix for BUG#35351 "Maria: R-tree index does not return all expected rows" BitKeeper/triggers/post-commit: commits to Maria public list mysql-test/r/maria-gis-rtree.result: result is good now, similar to MyISAM's (gis-rtree.result) storage/maria/ma_rt_index.c: R-tree key-reading code used info->buff as a cache for the next key read, but between key read and next key read, there is record read, which uses info->buff too. In detail, during a SELECT: First key read: maria_rfirst() is called, which calls maria_rtree_find_first() which calls maria_rtree_find_req() which comes here if (after_key < last) { // ! the list of keys is copied to info->buff // and info->buff is remembered in info->int_keypos info->int_keypos= info->buff; info->int_maxpos= info->buff + (last - after_key); memcpy(info->buff, after_key, last - after_key); info->keyread_buff_used= 0; } Then record read: _ma_read_block_record() (as well as some other functions of ma_blockrec.c) overwrites info->buff: if (!(buff= pagecache_read(share->pagecache, &info->dfile, ma_recordpos_to_page(record_pos), 0, info->buff, share->page_type, PAGECACHE_LOCK_LEFT_UNLOCKED, 0))) So, this has the effect that the keys saved by maria_rtree_find_req() are now lost: info->int_keypos now contains a copy of a data page! Then maria_rnext_same() runs (to find second row), calls maria_rtree_find_next() which does: if (!info->keyread_buff_used) { uchar *key= info->int_keypos; while (key < info->int_maxpos) { if (!maria_rtree_key_cmp(keyinfo->seg, info->first_mbr_key, key, info->last_rkey_length, search_flag)) i.e. maria_rtree_key_cmp() is doing comparisons on values it reads from the data page. Naturally this is bad and no row is found. Origin of the bug: MARIA_HA::keyread_buff is new in Maria. Solution: use keyread_buff instead of buff (like _ma_search_next() does), in R-tree code. Note that ma_blockrec.c functions also use keyread_buff but they all are write-functions, which should not be running close to next-key-read. Also note that some ma_rt_index.c functions still use info->buff, but they are about writes too. Thanks Monty for the idea. --- BitKeeper/triggers/post-commit | 6 +++--- mysql-test/r/maria-gis-rtree.result | 15 ++++++++++++++- storage/maria/ma_rt_index.c | 17 ++++++++++------- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/BitKeeper/triggers/post-commit b/BitKeeper/triggers/post-commit index abc9de5cb4e..e3cd589f50b 100755 --- a/BitKeeper/triggers/post-commit +++ b/BitKeeper/triggers/post-commit @@ -8,7 +8,7 @@ else COMMITTER=$USER fi FROM=$COMMITTER@mysql.com -COMMITS=commits@lists.mysql.com +COMMITS=maria@lists.mysql.com DOCS=docs-commit@mysql.com LIMIT=10000 VERSION="maria" @@ -73,11 +73,11 @@ else fi #++ -# commits@ or dev-private@ mail +# maria@ or dev-private@ mail #-- LIST="commits" -TO="commits@lists.mysql.com" +TO="maria@lists.mysql.com" if [ -f .tree-is-private ] then LIST="dev-private" diff --git a/mysql-test/r/maria-gis-rtree.result b/mysql-test/r/maria-gis-rtree.result index 749a5cc4a71..0c929fe1313 100644 --- a/mysql-test/r/maria-gis-rtree.result +++ b/mysql-test/r/maria-gis-rtree.result @@ -172,6 +172,16 @@ id select_type table type possible_keys key key_len ref rows Extra SELECT fid, AsText(g) FROM t1 WHERE Within(g, GeomFromText('Polygon((140 140,160 140,160 160,140 160,140 140))')); fid AsText(g) 1 LINESTRING(150 150,150 150) +2 LINESTRING(149 149,151 151) +3 LINESTRING(148 148,152 152) +4 LINESTRING(147 147,153 153) +5 LINESTRING(146 146,154 154) +6 LINESTRING(145 145,155 155) +7 LINESTRING(144 144,156 156) +8 LINESTRING(143 143,157 157) +9 LINESTRING(142 142,158 158) +10 LINESTRING(141 141,159 159) +11 LINESTRING(140 140,160 160) DROP TABLE t1; CREATE TABLE t2 ( fid INT NOT NULL AUTO_INCREMENT PRIMARY KEY, @@ -297,6 +307,9 @@ SELECT fid, AsText(g) FROM t2 WHERE Within(g, GeomFromText('Polygon((40 40,60 40,60 60,40 60,40 40))')); fid AsText(g) 45 LINESTRING(51 51,60 60) +46 LINESTRING(51 41,60 50) +55 LINESTRING(41 51,50 60) +56 LINESTRING(41 41,50 50) DELETE FROM t2 WHERE Within(g, Envelope(GeometryFromWKB(LineString(Point(10 * 10 - 9, 10 * 10 - 9), Point(10 * 10, 10 * 10))))); SELECT count(*) FROM t2; count(*) @@ -1469,7 +1482,7 @@ INSERT INTO t1 VALUES (2, GEOMFROMTEXT('LINESTRING(1102218.456 1,2000000 2)')); SELECT COUNT(*) FROM t1 WHERE MBRINTERSECTS(b, GEOMFROMTEXT('LINESTRING(1 1,1102219 2)') ); COUNT(*) -1 +2 SELECT COUNT(*) FROM t1 IGNORE INDEX (b) WHERE MBRINTERSECTS(b, GEOMFROMTEXT('LINESTRING(1 1,1102219 2)') ); COUNT(*) diff --git a/storage/maria/ma_rt_index.c b/storage/maria/ma_rt_index.c index 1608900d8f3..4f80d6a5a76 100644 --- a/storage/maria/ma_rt_index.c +++ b/storage/maria/ma_rt_index.c @@ -128,9 +128,10 @@ static int maria_rtree_find_req(MARIA_HA *info, MARIA_KEYDEF *keyinfo, if (after_key < last) { - info->int_keypos= info->buff; - info->int_maxpos= info->buff + (last - after_key); - memcpy(info->buff, after_key, last - after_key); + uchar *keyread_buff= info->keyread_buff; + info->int_keypos= keyread_buff; + info->int_maxpos= keyread_buff + (last - after_key); + memcpy(keyread_buff, after_key, last - after_key); info->keyread_buff_used= 0; } else @@ -346,9 +347,10 @@ static int maria_rtree_get_req(MARIA_HA *info, MARIA_KEYDEF *keyinfo, if (after_key < last) { + uchar *keyread_buff= info->keyread_buff; info->int_keypos= (uchar*) saved_key; - memcpy(info->buff, page_buf, keyinfo->block_length); - info->int_maxpos= rt_PAGE_END(share, info->buff); + memcpy(keyread_buff, page_buf, keyinfo->block_length); + info->int_maxpos= rt_PAGE_END(share, keyread_buff); info->keyread_buff_used= 0; } else @@ -415,12 +417,13 @@ int maria_rtree_get_next(MARIA_HA *info, uint keynr, uint key_length) { my_off_t root; MARIA_KEYDEF *keyinfo= info->s->keyinfo + keynr; + uchar *keyread_buff= info->keyread_buff; if (!info->keyread_buff_used) { uint k_len= keyinfo->keylength - info->s->base.rec_reflength; /* rt_PAGE_NEXT_KEY(info->int_keypos) */ - uchar *key= info->buff + *(int*)info->int_keypos + k_len + + uchar *key= keyread_buff + *(int*)info->int_keypos + k_len + info->s->base.rec_reflength; /* rt_PAGE_NEXT_KEY(key) */ uchar *after_key= key + k_len + info->s->base.rec_reflength; @@ -429,7 +432,7 @@ int maria_rtree_get_next(MARIA_HA *info, uint keynr, uint key_length) info->lastkey_length= k_len + info->s->base.rec_reflength; memcpy(info->lastkey, key, k_len + info->s->base.rec_reflength); - *(int*)info->int_keypos= key - info->buff; + *(int*)info->int_keypos= key - keyread_buff; if (after_key >= info->int_maxpos) { info->keyread_buff_used= 1;