Commit 28131daa authored by unknown's avatar unknown

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.
parent b7659974
...@@ -8,7 +8,7 @@ else ...@@ -8,7 +8,7 @@ else
COMMITTER=$USER COMMITTER=$USER
fi fi
FROM=$COMMITTER@mysql.com FROM=$COMMITTER@mysql.com
COMMITS=commits@lists.mysql.com COMMITS=maria@lists.mysql.com
DOCS=docs-commit@mysql.com DOCS=docs-commit@mysql.com
LIMIT=10000 LIMIT=10000
VERSION="maria" VERSION="maria"
...@@ -73,11 +73,11 @@ else ...@@ -73,11 +73,11 @@ else
fi fi
#++ #++
# commits@ or dev-private@ mail # maria@ or dev-private@ mail
#-- #--
LIST="commits" LIST="commits"
TO="commits@lists.mysql.com" TO="maria@lists.mysql.com"
if [ -f .tree-is-private ] if [ -f .tree-is-private ]
then then
LIST="dev-private" LIST="dev-private"
......
...@@ -172,6 +172,16 @@ id select_type table type possible_keys key key_len ref rows Extra ...@@ -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))')); SELECT fid, AsText(g) FROM t1 WHERE Within(g, GeomFromText('Polygon((140 140,160 140,160 160,140 160,140 140))'));
fid AsText(g) fid AsText(g)
1 LINESTRING(150 150,150 150) 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; DROP TABLE t1;
CREATE TABLE t2 ( CREATE TABLE t2 (
fid INT NOT NULL AUTO_INCREMENT PRIMARY KEY, fid INT NOT NULL AUTO_INCREMENT PRIMARY KEY,
...@@ -297,6 +307,9 @@ SELECT fid, AsText(g) FROM t2 WHERE Within(g, ...@@ -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))')); GeomFromText('Polygon((40 40,60 40,60 60,40 60,40 40))'));
fid AsText(g) fid AsText(g)
45 LINESTRING(51 51,60 60) 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))))); 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; SELECT count(*) FROM t2;
count(*) count(*)
...@@ -1469,7 +1482,7 @@ INSERT INTO t1 VALUES (2, GEOMFROMTEXT('LINESTRING(1102218.456 1,2000000 2)')); ...@@ -1469,7 +1482,7 @@ INSERT INTO t1 VALUES (2, GEOMFROMTEXT('LINESTRING(1102218.456 1,2000000 2)'));
SELECT COUNT(*) FROM t1 WHERE SELECT COUNT(*) FROM t1 WHERE
MBRINTERSECTS(b, GEOMFROMTEXT('LINESTRING(1 1,1102219 2)') ); MBRINTERSECTS(b, GEOMFROMTEXT('LINESTRING(1 1,1102219 2)') );
COUNT(*) COUNT(*)
1 2
SELECT COUNT(*) FROM t1 IGNORE INDEX (b) WHERE SELECT COUNT(*) FROM t1 IGNORE INDEX (b) WHERE
MBRINTERSECTS(b, GEOMFROMTEXT('LINESTRING(1 1,1102219 2)') ); MBRINTERSECTS(b, GEOMFROMTEXT('LINESTRING(1 1,1102219 2)') );
COUNT(*) COUNT(*)
......
...@@ -128,9 +128,10 @@ static int maria_rtree_find_req(MARIA_HA *info, MARIA_KEYDEF *keyinfo, ...@@ -128,9 +128,10 @@ static int maria_rtree_find_req(MARIA_HA *info, MARIA_KEYDEF *keyinfo,
if (after_key < last) if (after_key < last)
{ {
info->int_keypos= info->buff; uchar *keyread_buff= info->keyread_buff;
info->int_maxpos= info->buff + (last - after_key); info->int_keypos= keyread_buff;
memcpy(info->buff, after_key, last - after_key); info->int_maxpos= keyread_buff + (last - after_key);
memcpy(keyread_buff, after_key, last - after_key);
info->keyread_buff_used= 0; info->keyread_buff_used= 0;
} }
else else
...@@ -346,9 +347,10 @@ static int maria_rtree_get_req(MARIA_HA *info, MARIA_KEYDEF *keyinfo, ...@@ -346,9 +347,10 @@ static int maria_rtree_get_req(MARIA_HA *info, MARIA_KEYDEF *keyinfo,
if (after_key < last) if (after_key < last)
{ {
uchar *keyread_buff= info->keyread_buff;
info->int_keypos= (uchar*) saved_key; info->int_keypos= (uchar*) saved_key;
memcpy(info->buff, page_buf, keyinfo->block_length); memcpy(keyread_buff, page_buf, keyinfo->block_length);
info->int_maxpos= rt_PAGE_END(share, info->buff); info->int_maxpos= rt_PAGE_END(share, keyread_buff);
info->keyread_buff_used= 0; info->keyread_buff_used= 0;
} }
else else
...@@ -415,12 +417,13 @@ int maria_rtree_get_next(MARIA_HA *info, uint keynr, uint key_length) ...@@ -415,12 +417,13 @@ int maria_rtree_get_next(MARIA_HA *info, uint keynr, uint key_length)
{ {
my_off_t root; my_off_t root;
MARIA_KEYDEF *keyinfo= info->s->keyinfo + keynr; MARIA_KEYDEF *keyinfo= info->s->keyinfo + keynr;
uchar *keyread_buff= info->keyread_buff;
if (!info->keyread_buff_used) if (!info->keyread_buff_used)
{ {
uint k_len= keyinfo->keylength - info->s->base.rec_reflength; uint k_len= keyinfo->keylength - info->s->base.rec_reflength;
/* rt_PAGE_NEXT_KEY(info->int_keypos) */ /* 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; info->s->base.rec_reflength;
/* rt_PAGE_NEXT_KEY(key) */ /* rt_PAGE_NEXT_KEY(key) */
uchar *after_key= key + k_len + info->s->base.rec_reflength; 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) ...@@ -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; info->lastkey_length= k_len + info->s->base.rec_reflength;
memcpy(info->lastkey, key, 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) if (after_key >= info->int_maxpos)
{ {
info->keyread_buff_used= 1; info->keyread_buff_used= 1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment