List:Commits« Previous MessageNext Message »
From:Jorgen Loland Date:February 9 2012 11:22am
Subject:bzr push into mysql-trunk branch (jorgen.loland:3865 to 3866) Bug#13354910
View as plain text  
 3866 Jorgen Loland	2012-02-09
      Bug#13354910 - MAKE_JOIN_STATISTICS() INVOKES 
                     RECORDS_IN_RANGE(MIN_KEY=0X0, MAX_KEY=0X0)
      
      Some range predicates that by them selves make up partial 
      ranges are merged into a full range by the range optimizer. 
      An example is the query for this bug:
      
        "WHERE col IN (1,2,6) OR col NOT IN (1)"
      
      This predicate is always true and is correctly merged into
      the range [-inf <= col <= +inf]. However, although the range
      optimizer did make a full range, it did not realize that it's
      meaningless to use this as a range predicate since that will 
      be the same as doing an index scan.
      
      For the query in the bug the problem was that 
      SEL_ARG::copy_max() had a bug: When checking if the range was
      a full range, it was checked if max_flag was set to 
      (NO_MAX_RANGE | NO_MIN_RANGE). But the NO_MIN_FLAG is stored
      in min_flag, so the test is changed to 
      (max_flag & NO_MAX_FLAG) and (min_flag & NO_MIN_FLAG).
      
      Enabling the DBUG_ASSERT(max_key || min_key) in 
      ha_innobase::records_in_range() then showed another related 
      bug: innodb_icp started failing because SEL_ARG::copy_min()
      had the very same bug as copy_max(). This was fixed as well.
      
      In an attempt at unittesting SEL_ARG, copy_min() and copy_max()
      have now been added to the opt_range-t gunit test.
      
      To see the assert, add "DBUG_ASSERT(min_key || max_key);"
      to the beginning of ha_innobase::records_in_range().
     @ mysql-test/include/index_merge1.inc
        Added a new test to test the intended behavior of a
        query that changed due to the bugfix.
     @ mysql-test/r/index_merge_myisam.result
        Updated result file because an invalid execution plan changed.
        Also added a new test to test the intended behavior of the
        query that changed.
     @ sql/opt_range.cc
        SEL_ARG::copy_min() and copy_max() now checks if the NO_MIN_RANGE
        flag is set in min_flag and the NO_MAX_FLAG in max_flag.
     @ unittest/gunit/opt_range-t.cc
        Add tests for SEL_ARG::copy_min() and SEL_ARG::copy_max().

    modified:
      mysql-test/include/index_merge1.inc
      mysql-test/r/index_merge_myisam.result
      sql/opt_range.cc
      unittest/gunit/opt_range-t.cc
 3865 Guilhem Bichot	2012-02-08
      Fix for
      Bug #13688248 	CRASH IN DIAGNOSTICS_AREA::SET_OK_STATUS WHEN USING DEBUG_SYNC.
      And, to test the above, I had to fix
      Bug #13688015 	THE OPTION --LOOSE-DEBUG-SYNC-TIMEOUT IS PLACED WRONG
     @ mysql-test/mysql-test-run.pl
        Options specified in .opt files should be added last so they can
        override defaults of mtr (fixes Bug#13688015)
     @ mysql-test/t/debug_sync2.test
        test for bug, used to assert
     @ sql/debug_sync.cc
        In the scenario of debug_sync2.test, debug sync point timed out;
        thus a warning was raised, but because sql_mode=traditional,
        push_warning() changed it to an error. But code in mysql_insert()
        (container of the debug sync point) didn't handle this error,
        finally an OK was sent, which triggered an assertion (you can't
        have an OK *and* an error at the same time).
        Fix: warnings of debug_sync should never be errors (sql_mode=traditional
        is, per the doc, for *bad data* errors; a debug sync timeout
        will not cause bad data; and debug_sync is only meant for our QA and devs,
        and we only ship it in debug builds).
        So: disable abort_on_warning.

    added:
      mysql-test/r/debug_sync2.result
      mysql-test/t/debug_sync2-master.opt
      mysql-test/t/debug_sync2.test
    modified:
      mysql-test/mysql-test-run.pl
      sql/debug_sync.cc
=== modified file 'mysql-test/include/index_merge1.inc'
--- a/mysql-test/include/index_merge1.inc	2011-11-03 07:01:49 +0000
+++ b/mysql-test/include/index_merge1.inc	2012-02-09 11:22:17 +0000
@@ -167,8 +167,14 @@ explain select * from t0 where
 explain select * from t0 where
     ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4))
   or
-    ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6));
+    ((key3 >5 or key5 < 2) and (key5 < 5 or key6 < 6));
+
+explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where
+    ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4))
+  or
+    ((key3 >5 or key5 < 2) and (key5 < 5 or key6 < 6));
 
+# Can't merge any indexes here (predicate on key3 is always true)
 explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where
     ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4))
   or

=== modified file 'mysql-test/r/index_merge_myisam.result'
--- a/mysql-test/r/index_merge_myisam.result	2012-01-30 13:13:15 +0000
+++ b/mysql-test/r/index_merge_myisam.result	2012-02-09 11:22:17 +0000
@@ -170,15 +170,21 @@ id	select_type	table	type	possible_keys	
 explain select * from t0 where
 ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4))
 or
-((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6));
+((key3 >5 or key5 < 2) and (key5 < 5 or key6 < 6));
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t0	ALL	i1,i2,i3,i5,i6	NULL	NULL	NULL	1024	Using where
 explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where
 ((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4))
 or
+((key3 >5 or key5 < 2) and (key5 < 5 or key6 < 6));
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t0	index_merge	i1,i2,i3,i5,i6	i3,i5	4,4	NULL	1024	Using sort_union(i3,i5); Using where
+explain select * from t0 force index(i1, i2, i3, i4, i5, i6 ) where
+((key3 <5 or key5 < 4) and (key1 < 4 or key2 < 4))
+or
 ((key3 >=5 or key5 < 2) and (key5 < 5 or key6 < 6));
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
-1	SIMPLE	t0	index_merge	i1,i2,i3,i5,i6	i3,i5	0,4	NULL	1024	Using sort_union(i3,i5); Using where
+1	SIMPLE	t0	ALL	i1,i2,i3,i5,i6	NULL	NULL	NULL	1024	Using where
 select * from t0 where key1 < 5 or key8 < 4 order by key1;
 key1	key2	key3	key4	key5	key6	key7	key8
 1	1	1	1	1	1	1	1023

=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2012-02-09 09:09:58 +0000
+++ b/sql/opt_range.cc	2012-02-09 11:22:17 +0000
@@ -483,8 +483,7 @@ public:
     if (cmp_min_to_min(arg) > 0)
     {
       min_value=arg->min_value; min_flag=arg->min_flag;
-      if ((max_flag & (NO_MAX_RANGE | NO_MIN_RANGE)) ==
-	  (NO_MAX_RANGE | NO_MIN_RANGE))
+      if ((max_flag & NO_MAX_RANGE) && (min_flag & NO_MIN_RANGE))
 	return 1;				// Full range
     }
     maybe_flag|=arg->maybe_flag;
@@ -495,8 +494,7 @@ public:
     if (cmp_max_to_max(arg) <= 0)
     {
       max_value=arg->max_value; max_flag=arg->max_flag;
-      if ((max_flag & (NO_MAX_RANGE | NO_MIN_RANGE)) ==
-	  (NO_MAX_RANGE | NO_MIN_RANGE))
+      if ((max_flag & NO_MAX_RANGE) && (min_flag & NO_MIN_RANGE))
 	return 1;				// Full range
     }
     maybe_flag|=arg->maybe_flag;
@@ -7475,7 +7473,16 @@ key_or(RANGE_OPT_PARAM *param, SEL_ARG *
           cur_key2->next= next_key2;                 // New copy of cur_key2
         }
 
-        cur_key2->copy_min(cur_key1);
+        if (cur_key2->copy_min(cur_key1))
+        {
+          // cur_key2 is full range: [-inf <= cur_key2 <= +inf]
+          key1->free_tree();
+          key2->free_tree();
+          if (key1->maybe_flag)
+            return new SEL_ARG(SEL_ARG::MAYBE_KEY);
+          return 0;
+        }
+
         if (!(key1= key1->tree_delete(cur_key1)))
         {
           /*

=== modified file 'unittest/gunit/opt_range-t.cc'
--- a/unittest/gunit/opt_range-t.cc	2011-12-20 09:51:05 +0000
+++ b/unittest/gunit/opt_range-t.cc	2012-02-09 11:22:17 +0000
@@ -48,6 +48,7 @@ protected:
     initializer.SetUp();
     m_opt_param.thd= thd();
     m_opt_param.mem_root= &m_alloc;
+    m_opt_param.current_table= 1<<0;
     init_sql_alloc(&m_alloc, thd()->variables.range_alloc_block_size, 0);
   }
 
@@ -102,7 +103,7 @@ const SEL_TREE *null_tree= NULL;
 class Mock_field_long : public Field_long
 {
 public:
-  Mock_field_long()
+  Mock_field_long(THD *thd, Item *item)
     : Field_long(0,                             // ptr_arg
                  8,                             // len_arg
                  NULL,                          // null_ptr_arg
@@ -118,30 +119,313 @@ public:
     m_share.db.str= const_cast<char*>(foo);
     m_share.db.length= strlen(m_share.db.str);
 
+    bitmap_init(&share_allset, 0, sizeof(my_bitmap_map), 0);
+    bitmap_set_above(&share_allset, 0, 1); //all bits 1
+    m_share.all_set= share_allset;
+
     memset(&m_table, 0, sizeof(m_table));
     m_table.s= &m_share;
+
+    bitmap_init(&tbl_readset, 0, sizeof(my_bitmap_map), 0);
+    m_table.read_set= &tbl_readset;
+
+    bitmap_init(&tbl_writeset, 0, sizeof(my_bitmap_map), 0);
+    m_table.write_set= &tbl_writeset;
+
+    m_table.map= 1<<0;
+    m_table.in_use= thd;
     this->table_name= &m_table_name;
     this->table= &m_table;
+    this->ptr= (uchar*) alloc_root((thd->mem_root), KEY_LENGTH);
+    if (item)
+      item->save_in_field_no_warnings(this, true);      
   }
+  
+  // #bytes to store the value - see Field_long::key_lenght()
+  static const int KEY_LENGTH= 4;
   const char *m_table_name;
   TABLE_SHARE m_share;
   TABLE       m_table;
+  MY_BITMAP   share_allset;
+  MY_BITMAP   tbl_readset;
+  MY_BITMAP   tbl_writeset;
 };
 
 
+class Debug_sel_arg : public SEL_ARG
+{
+public:
+  Debug_sel_arg(Field *f, const uchar *min_val, const uchar *max_val, 
+               const KEY_PART_INFO *kpi_)
+    : SEL_ARG(f, min_val, max_val), kpi(kpi_)
+  {}
+
+  void print(String *s)
+  {
+    append_range(s, kpi, min_value, max_value, min_flag | max_flag);
+  }
+private:
+  const KEY_PART_INFO * const kpi;
+};
+
 TEST_F(SelArgTest, SimpleCond)
 {
   EXPECT_NE(null_tree, get_mm_tree(&m_opt_param, new Item_int(42)));
 }
 
 
+/*
+  TODO: Here we try to build a range, but a lot of mocking remains
+  before it works as intended. Currently get_mm_tree() returns NULL
+  because m_opt_param.key_parts and m_opt_param.key_parts_end have not
+  been setup. 
+*/
 TEST_F(SelArgTest, EqualCond)
 {
-  Mock_field_long field_long;
-  EXPECT_EQ(null_tree,
-            get_mm_tree(&m_opt_param,
-                        new Item_equal(new Item_int(42),
-                                       new Item_field(&field_long))));
+  Mock_field_long field_long(thd(), NULL);
+  m_opt_param.table= &field_long.m_table;
+  SEL_TREE *tree= get_mm_tree(&m_opt_param,
+                              new Item_equal(new Item_int(42),
+                                             new Item_field(&field_long)));
+  EXPECT_EQ(null_tree, tree);
+}
+
+
+TEST_F(SelArgTest, SelArgOnevalue)
+{
+  Mock_field_long field_long7(thd(), new Item_int(7));
+
+  KEY_PART_INFO kpi;
+  kpi.init_from_field(&field_long7);
+
+  uchar range_val7[field_long7.KEY_LENGTH];
+  field_long7.get_key_image(range_val7, kpi.length, Field::itRAW);
+
+  Debug_sel_arg sel_arg7(&field_long7, range_val7, range_val7, &kpi);
+  String range_string;
+  sel_arg7.print(&range_string);
+  const char expected[]= "7 <= field_name <= 7";
+  EXPECT_STREQ(expected, range_string.c_ptr());
+
+  sel_arg7.min_flag|= NO_MIN_RANGE;
+  range_string.length(0);
+  sel_arg7.print(&range_string);
+  const char expected2[]= "field_name <= 7";
+  EXPECT_STREQ(expected2, range_string.c_ptr());
+
+  sel_arg7.max_flag= NEAR_MAX;
+  range_string.length(0);
+  sel_arg7.print(&range_string);
+  const char expected3[]= "field_name < 7";
+  EXPECT_STREQ(expected3, range_string.c_ptr());
+
+  sel_arg7.min_flag= NEAR_MIN;
+  sel_arg7.max_flag= NO_MAX_RANGE;
+  range_string.length(0);
+  sel_arg7.print(&range_string);
+  const char expected4[]= "7 < field_name";
+  EXPECT_STREQ(expected4, range_string.c_ptr());
+
+  sel_arg7.min_flag= 0;
+  range_string.length(0);
+  sel_arg7.print(&range_string);
+  const char expected5[]= "7 <= field_name";
+  EXPECT_STREQ(expected5, range_string.c_ptr());
+}
+
+
+TEST_F(SelArgTest, SelArgBetween)
+{
+  Mock_field_long field_long3(thd(), new Item_int(3));
+  Mock_field_long field_long5(thd(), new Item_int(5));
+
+  KEY_PART_INFO kpi;
+  kpi.init_from_field(&field_long3);
+
+  uchar range_val3[field_long3.KEY_LENGTH];
+  field_long3.get_key_image(range_val3, kpi.length, Field::itRAW);
+
+  uchar range_val5[field_long5.KEY_LENGTH];
+  field_long5.get_key_image(range_val5, kpi.length, Field::itRAW);
+
+  Debug_sel_arg sel_arg35(&field_long3, range_val3, range_val5, &kpi);
+
+  String range_string;
+  sel_arg35.print(&range_string);
+  const char expected[]= "3 <= field_name <= 5";
+  EXPECT_STREQ(expected, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg35.min_flag= NEAR_MIN;
+  sel_arg35.print(&range_string);
+  const char expected2[]= "3 < field_name <= 5";
+  EXPECT_STREQ(expected2, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg35.max_flag= NEAR_MAX;
+  sel_arg35.print(&range_string);
+  const char expected3[]= "3 < field_name < 5";
+  EXPECT_STREQ(expected3, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg35.min_flag= 0;
+  sel_arg35.print(&range_string);
+  const char expected4[]= "3 <= field_name < 5";
+  EXPECT_STREQ(expected4, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg35.min_flag= NO_MIN_RANGE;
+  sel_arg35.max_flag= 0;
+  sel_arg35.print(&range_string);
+  const char expected5[]= "field_name <= 5";
+  EXPECT_STREQ(expected5, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg35.min_flag= 0;
+  sel_arg35.max_flag= NO_MAX_RANGE;
+  sel_arg35.print(&range_string);
+  const char expected6[]= "3 <= field_name";
+  EXPECT_STREQ(expected6, range_string.c_ptr());
+}
+
+TEST_F(SelArgTest, CopyMax)
+{
+  Mock_field_long field_long3(thd(), new Item_int(3));
+  Mock_field_long field_long5(thd(), new Item_int(5));
+
+  KEY_PART_INFO kpi;
+  kpi.init_from_field(&field_long3);
+
+  uchar range_val3[field_long3.KEY_LENGTH];
+  field_long3.get_key_image(range_val3, kpi.length, Field::itRAW);
+
+  uchar range_val5[field_long5.KEY_LENGTH];
+  field_long5.get_key_image(range_val5, kpi.length, Field::itRAW);
+
+  Debug_sel_arg sel_arg3(&field_long3, range_val3, range_val3, &kpi);
+  sel_arg3.min_flag= NO_MIN_RANGE;
+  Debug_sel_arg sel_arg5(&field_long5, range_val5, range_val5, &kpi);
+  sel_arg5.min_flag= NO_MIN_RANGE;
+
+  String range_string;
+  sel_arg3.print(&range_string);
+  const char expected[]= "field_name <= 3";
+  EXPECT_STREQ(expected, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg5.print(&range_string);
+  const char expected2[]= "field_name <= 5";
+  EXPECT_STREQ(expected2, range_string.c_ptr());
+
+  /*
+    Ranges now:
+                       -inf ----------------3-5----------- +inf
+    sel_arg3:          [-------------------->
+    sel_arg5:          [---------------------->
+    Below: merge these two ranges into sel_arg3 using copy_max()
+  */
+  bool full_range= sel_arg3.copy_max(&sel_arg5);
+  // The merged range does not cover all possible values
+  EXPECT_FALSE(full_range);
+
+  range_string.length(0);
+  sel_arg3.print(&range_string);
+  const char expected3[]= "field_name <= 5";
+  EXPECT_STREQ(expected3, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg5.min_flag= 0;
+  sel_arg5.max_flag= NO_MAX_RANGE;
+  sel_arg5.print(&range_string);
+  const char expected4[]= "5 <= field_name";
+  EXPECT_STREQ(expected4, range_string.c_ptr());
+
+  /*
+    Ranges now:
+                       -inf ----------------3-5----------- +inf
+    sel_arg3:          [---------------------->
+    sel_arg5:                                 <---------------]
+    Below: merge these two ranges into sel_arg3 using copy_max()
+  */
+
+  full_range= sel_arg3.copy_max(&sel_arg5);
+  // The new range covers all possible values
+  EXPECT_TRUE(full_range);
+
+  range_string.length(0);
+  sel_arg3.print(&range_string);
+  const char expected5[]= "field_name";
+  EXPECT_STREQ(expected5, range_string.c_ptr());
+}
+
+TEST_F(SelArgTest, CopyMin)
+{
+  Mock_field_long field_long3(thd(), new Item_int(3));
+  Mock_field_long field_long5(thd(), new Item_int(5));
+
+  KEY_PART_INFO kpi;
+  kpi.init_from_field(&field_long3);
+
+  uchar range_val3[field_long3.KEY_LENGTH];
+  field_long3.get_key_image(range_val3, kpi.length, Field::itRAW);
+
+  uchar range_val5[field_long5.KEY_LENGTH];
+  field_long5.get_key_image(range_val5, kpi.length, Field::itRAW);
+
+  Debug_sel_arg sel_arg3(&field_long3, range_val3, range_val3, &kpi);
+  sel_arg3.max_flag= NO_MAX_RANGE;
+  Debug_sel_arg sel_arg5(&field_long5, range_val5, range_val5, &kpi);
+  sel_arg5.max_flag= NO_MAX_RANGE;
+
+  String range_string;
+  sel_arg3.print(&range_string);
+  const char expected[]= "3 <= field_name";
+  EXPECT_STREQ(expected, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg5.print(&range_string);
+  const char expected2[]= "5 <= field_name";
+  EXPECT_STREQ(expected2, range_string.c_ptr());
+
+  /*
+    Ranges now:
+                       -inf ----------------3-5----------- +inf
+    sel_arg3:                               <-----------------]
+    sel_arg5:                                 <---------------]
+    Below: merge these two ranges into sel_arg3 using copy_max()
+  */
+  bool full_range= sel_arg5.copy_min(&sel_arg3);
+  // The merged range does not cover all possible values
+  EXPECT_FALSE(full_range);
+
+  range_string.length(0);
+  sel_arg5.print(&range_string);
+  const char expected3[]= "3 <= field_name";
+  EXPECT_STREQ(expected3, range_string.c_ptr());
+
+  range_string.length(0);
+  sel_arg3.max_flag= 0;
+  sel_arg3.min_flag= NO_MIN_RANGE;
+  sel_arg3.print(&range_string);
+  const char expected4[]= "field_name <= 3";
+  EXPECT_STREQ(expected4, range_string.c_ptr());
+
+  /*
+    Ranges now:
+                       -inf ----------------3-5----------- +inf
+    sel_arg3:          [-------------------->                
+    sel_arg5:                               <-----------------]
+    Below: merge these two ranges into sel_arg5 using copy_min()
+  */
+
+  full_range= sel_arg5.copy_min(&sel_arg3);
+  // The new range covers all possible values
+  EXPECT_TRUE(full_range);
+
+  range_string.length(0);
+  sel_arg5.print(&range_string);
+  const char expected5[]= "field_name";
+  EXPECT_STREQ(expected5, range_string.c_ptr());
 }
 
 }

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (jorgen.loland:3865 to 3866) Bug#13354910Jorgen Loland10 Feb