MySQL Bugs: #35351: Maria: R-tree index does not return all expected rows

Bug #35351	Maria: R-tree index does not return all expected rows
Submitted:	17 Mar 2008 21:30	Modified:	14 Apr 2008 12:02
Reporter:	Guilhem Bichot	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Maria storage engine	Severity:	S3 (Non-critical)
Version:	5.1-maria	OS:	Any
Assigned to:	Guilhem Bichot	CPU Architecture:	Any

Description:
After fixing BUG#35273 we can now run a GIS test on Maria: maria-gis-rtree.result unfortunately differs from gis-rtree.result in several places. At least the last difference is a real bug. It is not normal, not due to block size. If you change maria-gis-rtree.test to use row_format=dynamic instead of row_format=page, all bad differences go away.

How to repeat:
compare the two result files.

Suggested fix:
will post debugging info soon

Debugging info explaining the bug which causes the last difference between result files: the SELECT using R-tree index returns one row and not two because:
- maria_rfirst() is called, which calls maria_rtree_find_first() which calls 
maria_rtree_find_req(), which comes here:
    else
    {
      /* this is a leaf */
      if (!maria_rtree_key_cmp(keyinfo->seg, info->first_mbr_key,
                               k, info->last_rkey_length, search_flag))
      {
        uchar *after_key= (uchar*) rt_PAGE_NEXT_KEY(share, k, k_len, nod_flag);
        info->cur_row.lastpos= _ma_dpos(info, 0, after_key);
        info->lastkey_length= k_len + share->base.rec_reflength;
        memcpy(info->lastkey, k, info->lastkey_length);
        info->maria_rtree_recursion_depth= level;
        *saved_key= last - page_buf;

        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;
        }
So, ok, first row is found: it's now time to read it with _ma_read_block_record().
The problem is that _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.

This is a case of two modules using info->buff: rfirst places data into info->buff for rnext to use later, but read_record comes in the middle and overwrites info->buff.
Note: some other functions of ma_blockrec.c use info->buff too.
And while we are at it, it is also necessary to check usages of info->keyread_buff as it could have similar problems. Finally, please clarify when info->keyread_buff_used should be set to 1 (I see places which set info->keyread_buff but not info->keyread_buff_used=1 apparently), and why info->keread_buff_used is used in tests (is this robust).

The workaround is to use ROW_FORMAT=X where X is not PAGE (DYNAMIC, for example)

All abnormal differences between gis-rtree.result and maria-gis-rtree.result go away if at the start of _ma_read_block_record() one does:
  uchar *save_info_buff= info->buff;
  info->buff=my_malloc(share->block_size, MYF(0));
and at the end of this function, do:
  int res= _ma_read_block_record2(info, record, data, end_of_data);
  my_free(info->buff, MYF(0));
  info->buff= save_info_buff;
  DBUG_RETURN(res);
This is of course an inefficient solution, it's only to show that the conflicting use of info->buff by _ma_read_block_record() is the single cause of test's problems.
There may however also be problems (not revealed by the test) due to other functions of ma_blockrec.c using info->buff.

and all my questions about info->keyread_buff[_used] also remain.

Thank you for your bug report. This issue has been committed to our source repository of that product and will be incorporated into the next release.

If necessary, you can access the source repository and build the latest available version, including the bug fix. More information about accessing the source trees is available at

    http://dev.mysql.com/doc/en/installing-source.html