List:Maria Storage Engine« Previous MessageNext Message »
From:Guilhem Bichot Date:April 11 2008 9:53am
Subject:bk commit into maria tree (guilhem:1.2630) BUG#35351
View as plain text  
Below is the list of changes that have just been committed into a local
maria repository of guilhem.  When guilhem does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-04-11 11:53:21+02:00, guilhem@stripped +3 -0
  Fix for BUG#35351 "Maria: R-tree index does not return all expected rows"

  BitKeeper/triggers/post-commit@stripped, 2008-04-11 11:53:18+02:00, guilhem@stripped +3 -3
    commits to Maria public list

  mysql-test/r/maria-gis-rtree.result@stripped, 2008-04-11 11:53:19+02:00, guilhem@stripped +14 -1
    result is good now, similar to MyISAM's (gis-rtree.result)

  storage/maria/ma_rt_index.c@stripped, 2008-04-11 11:53:19+02:00, guilhem@stripped +10 -7
    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.

diff -Nrup a/BitKeeper/triggers/post-commit b/BitKeeper/triggers/post-commit
--- a/BitKeeper/triggers/post-commit	2008-02-05 16:47:05 +01:00
+++ b/BitKeeper/triggers/post-commit	2008-04-11 11:53:18 +02:00
@@ -8,7 +8,7 @@ else
   COMMITTER=$USER
 fi
 FROM=$COMMITTER@stripped
-COMMITS=commits@stripped
+COMMITS=maria@stripped
 DOCS=docs-commit@stripped
 LIMIT=10000
 VERSION="maria"
@@ -73,11 +73,11 @@ else
 fi
 
 #++
-# commits@ or dev-private@ mail
+# maria@ or dev-private@ mail
 #--
 
 LIST="commits"
-TO="commits@stripped"
+TO="maria@stripped"
 if [ -f .tree-is-private ]
 then
   LIST="dev-private"
diff -Nrup a/mysql-test/r/maria-gis-rtree.result b/mysql-test/r/maria-gis-rtree.result
--- a/mysql-test/r/maria-gis-rtree.result	2008-03-17 22:31:48 +01:00
+++ b/mysql-test/r/maria-gis-rtree.result	2008-04-11 11:53:19 +02:00
@@ -172,6 +172,16 @@ id	select_type	table	type	possible_keys	
 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 With
 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('
 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 -Nrup a/storage/maria/ma_rt_index.c b/storage/maria/ma_rt_index.c
--- a/storage/maria/ma_rt_index.c	2008-04-03 15:40:20 +02:00
+++ b/storage/maria/ma_rt_index.c	2008-04-11 11:53:19 +02:00
@@ -128,9 +128,10 @@ static int maria_rtree_find_req(MARIA_HA
 
         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 
 
       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,
 {
   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,
     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;
Thread
bk commit into maria tree (guilhem:1.2630) BUG#35351Guilhem Bichot11 Apr