[Commits] Rev 2846: Code cleanup in file:///home/psergey/dev2/maria-5.3-mwl128-dsmrr-cpk/

Sergey Petrunya psergey at askmonty.org
Sun Nov 7 17:21:31 EET 2010


At file:///home/psergey/dev2/maria-5.3-mwl128-dsmrr-cpk/

------------------------------------------------------------
revno: 2846
revision-id: psergey at askmonty.org-20101107152127-xrb2yx1hfl84et9c
parent: psergey at askmonty.org-20101103080307-iamckpsgu3ncjqvq
committer: Sergey Petrunya <psergey at askmonty.org>
branch nick: maria-5.3-mwl128-dsmrr-cpk
timestamp: Sun 2010-11-07 18:21:27 +0300
message:
  Code cleanup
=== modified file 'mysql-test/r/myisam_mrr.result'
--- a/mysql-test/r/myisam_mrr.result	2010-11-02 16:32:26 +0000
+++ b/mysql-test/r/myisam_mrr.result	2010-11-07 15:21:27 +0000
@@ -462,3 +462,60 @@
 WHERE `key1` LIKE CONCAT( LEFT( '1' , 7 ) , '%' )
 ORDER BY col1 LIMIT 7;
 drop table t0, t1, t2;
+#
+# BUG#670417: Diverging results in maria-5.3-mwl128-dsmrr-cpk with join buffer (incremental, BKA join)
+#
+set @save_join_cache_level = @@join_cache_level;
+set join_cache_level = 6;
+set @save_join_buffer_size=@@join_buffer_size;
+set join_buffer_size = 136;
+Warnings:
+Warning	1292	Truncated incorrect join_buffer_size value: '136'
+CREATE TABLE `t1` (
+`pk` int(11) NOT NULL AUTO_INCREMENT,
+`col_int_key` int(11) NOT NULL,
+`col_varchar_key` varchar(1) NOT NULL,
+`col_varchar_nokey` varchar(1) NOT NULL,
+PRIMARY KEY (`pk`),
+KEY `col_int_key` (`col_int_key`),
+KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
+) ENGINE=MyISAM AUTO_INCREMENT=30 DEFAULT CHARSET=latin1;
+INSERT INTO t1 VALUES 
+(10,8,'v','v'), (11,8,'f','f'), (12,5,'v','v'),
+(13,8,'s','s'),(14,8,'a','a'),(15,6,'p','p'),
+(16,7,'z','z'),(17,2,'a','a'),(18,5,'h','h'),
+(19,7,'h','h'),(20,2,'v','v'),(21,9,'v','v'),
+(22,142,'b','b'),(23,3,'y','y'),(24,0,'v','v'),
+(25,3,'m','m'),(26,5,'z','z'),(27,9,'n','n'),
+(28,1,'d','d'),(29,107,'a','a');
+CREATE TABLE `t2` (
+`pk` int(11) NOT NULL AUTO_INCREMENT,
+`col_int_key` int(11) NOT NULL,
+`col_varchar_key` varchar(1) NOT NULL,
+`col_varchar_nokey` varchar(1) NOT NULL,
+PRIMARY KEY (`pk`),
+KEY `col_int_key` (`col_int_key`),
+KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
+) ENGINE=MyISAM AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
+INSERT INTO `t2` VALUES (1,9,'x','x');
+SELECT 
+COUNT(table2.col_varchar_key) 
+FROM 
+t2 STRAIGHT_JOIN ( t1 AS table2 JOIN t1 AS table3 ON table3.pk ) 
+ON table3.col_varchar_key = table2.col_varchar_key AND 
+table3.col_varchar_key = table2.col_varchar_nokey;
+COUNT(table2.col_varchar_key)
+49
+EXPLAIN SELECT 
+COUNT(table2.col_varchar_key) 
+FROM 
+t2 STRAIGHT_JOIN ( t1 AS table2 JOIN t1 AS table3 ON table3.pk ) 
+ON table3.col_varchar_key = table2.col_varchar_key AND 
+table3.col_varchar_key = table2.col_varchar_nokey;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	system	NULL	NULL	NULL	NULL	1	
+1	SIMPLE	table2	ALL	col_varchar_key	NULL	NULL	NULL	20	Using where
+1	SIMPLE	table3	ref	col_varchar_key	col_varchar_key	3	test.table2.col_varchar_key	3	Using index condition(BKA); Using where; Using join buffer (flat, BKA join)
+set join_cache_level= @save_join_cache_level;
+set join_buffer_size= @save_join_buffer_size;
+drop table t1,t2;

=== modified file 'mysql-test/t/myisam_mrr.test'
--- a/mysql-test/t/myisam_mrr.test	2010-09-15 12:14:19 +0000
+++ b/mysql-test/t/myisam_mrr.test	2010-11-07 15:21:27 +0000
@@ -169,3 +169,61 @@
 WHERE `key1` LIKE CONCAT( LEFT( '1' , 7 ) , '%' )
 ORDER BY col1 LIMIT 7;
 drop table t0, t1, t2;
+
+--echo #
+--echo # BUG#670417: Diverging results in maria-5.3-mwl128-dsmrr-cpk with join buffer (incremental, BKA join)
+--echo #
+
+set @save_join_cache_level = @@join_cache_level;
+set join_cache_level = 6;
+set @save_join_buffer_size=@@join_buffer_size;
+set join_buffer_size = 136;
+
+CREATE TABLE `t1` (
+  `pk` int(11) NOT NULL AUTO_INCREMENT,
+  `col_int_key` int(11) NOT NULL,
+  `col_varchar_key` varchar(1) NOT NULL,
+  `col_varchar_nokey` varchar(1) NOT NULL,
+  PRIMARY KEY (`pk`),
+  KEY `col_int_key` (`col_int_key`),
+  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
+) ENGINE=MyISAM AUTO_INCREMENT=30 DEFAULT CHARSET=latin1;
+INSERT INTO t1 VALUES 
+  (10,8,'v','v'), (11,8,'f','f'), (12,5,'v','v'),
+  (13,8,'s','s'),(14,8,'a','a'),(15,6,'p','p'),
+  (16,7,'z','z'),(17,2,'a','a'),(18,5,'h','h'),
+  (19,7,'h','h'),(20,2,'v','v'),(21,9,'v','v'),
+  (22,142,'b','b'),(23,3,'y','y'),(24,0,'v','v'),
+  (25,3,'m','m'),(26,5,'z','z'),(27,9,'n','n'),
+  (28,1,'d','d'),(29,107,'a','a');
+
+CREATE TABLE `t2` (
+  `pk` int(11) NOT NULL AUTO_INCREMENT,
+  `col_int_key` int(11) NOT NULL,
+  `col_varchar_key` varchar(1) NOT NULL,
+  `col_varchar_nokey` varchar(1) NOT NULL,
+  PRIMARY KEY (`pk`),
+  KEY `col_int_key` (`col_int_key`),
+  KEY `col_varchar_key` (`col_varchar_key`,`col_int_key`)
+) ENGINE=MyISAM AUTO_INCREMENT=21 DEFAULT CHARSET=latin1;
+
+INSERT INTO `t2` VALUES (1,9,'x','x');
+
+SELECT 
+  COUNT(table2.col_varchar_key) 
+FROM 
+  t2 STRAIGHT_JOIN ( t1 AS table2 JOIN t1 AS table3 ON table3.pk ) 
+    ON table3.col_varchar_key = table2.col_varchar_key AND 
+       table3.col_varchar_key = table2.col_varchar_nokey;
+
+EXPLAIN SELECT 
+  COUNT(table2.col_varchar_key) 
+FROM 
+  t2 STRAIGHT_JOIN ( t1 AS table2 JOIN t1 AS table3 ON table3.pk ) 
+    ON table3.col_varchar_key = table2.col_varchar_key AND 
+       table3.col_varchar_key = table2.col_varchar_nokey;
+
+set join_cache_level= @save_join_cache_level;
+set join_buffer_size= @save_join_buffer_size;
+drop table t1,t2;
+

=== modified file 'sql/multi_range_read.cc'
--- a/sql/multi_range_read.cc	2010-11-03 08:03:07 +0000
+++ b/sql/multi_range_read.cc	2010-11-07 15:21:27 +0000
@@ -292,13 +292,14 @@
 {
   HANDLER_BUFFER no_buffer = {NULL, NULL, NULL};
   h= h_arg;
-  res= 0;
   return h->handler::multi_range_read_init(seq_funcs, seq_init_param, n_ranges,
                                            mode, &no_buffer);
 }
 
+
 int Mrr_simple_index_reader::get_next(char **range_info)
 {
+  int res;
   while (!(res= h->handler::multi_range_read_next(range_info)))
   {
     KEY_MULTI_RANGE *curr_range= &h->handler::mrr_cur_range;
@@ -344,6 +345,7 @@
     bool have_record= FALSE;
     if (scanning_key_val_iter)
     {
+      //psergey-todo: error handling!
       if (kv_it.get_next())
       {
         kv_it.close();
@@ -358,7 +360,6 @@
       {
         if (key_buffer->is_empty())
         {
-          index_scan_eof= TRUE;
           DBUG_RETURN(HA_ERR_END_OF_FILE);
         }
       }
@@ -366,11 +367,8 @@
     }
 
     if (have_record &&
-        (!mrr_funcs.skip_index_tuple ||
-         !mrr_funcs.skip_index_tuple(mrr_iter, *(char**)cur_range_info))
-        && 
-        (!mrr_funcs.skip_record ||
-         !mrr_funcs.skip_record(mrr_iter, *(char**)cur_range_info, NULL)))
+        !skip_index_tuple(*(char**)cur_range_info) &&
+        !skip_record(*(char**)cur_range_info, NULL))
     {
       break;
     }
@@ -448,7 +446,6 @@
 
   bool no_more_keys= test(res);
   scanning_key_val_iter= FALSE;
-  index_scan_eof= FALSE;
 
   if (no_more_keys && (!know_key_tuple_params || key_buffer->is_empty()))
     DBUG_RETURN(HA_ERR_END_OF_FILE);
@@ -586,7 +583,7 @@
 
 
 /*
-  Get the next record+range_id using ordered array of rowid+range_id pairds
+  Get the next {record, range_id} using ordered array of rowid+range_id pairs
 
   @note
     Since we have sorted rowids, we try not to make multiple rnd_pos() calls
@@ -976,7 +973,8 @@
 }
 
 
-int Mrr_ordered_index_reader::key_tuple_cmp_reverse(void* arg, uchar* key1, uchar* key2)
+int Mrr_ordered_index_reader::key_tuple_cmp_reverse(void* arg, uchar* key1, 
+                                                    uchar* key2)
 {
   return -key_tuple_cmp(arg, key1, key2);
 }
@@ -1070,11 +1068,11 @@
 }
 
 
-/**
+/*
   Take unused space from the key buffer and give it to the rowid buffer
 */
-//psergey-todo: do invoke this function.
-void DsMrr_impl::reallocate_buffer_space()
+
+void DsMrr_impl::redistribute_buffer_space()
 {
   uchar *unused_start, *unused_end;
   key_buffer->remove_unused_space(&unused_start, &unused_end);
@@ -1082,6 +1080,15 @@
 }
 
 
+/*
+  @brief Initialize the iterator
+  
+  @note
+  to point at the start of owner_arg->key_buffer
+
+  @retval .
+
+*/
 bool Key_value_records_iterator::init(Mrr_ordered_index_reader *owner_arg)
 {
   int res;
@@ -1089,9 +1096,11 @@
 
   identical_key_it.init(owner->key_buffer);
   /* Get the first pair into (cur_index_tuple, cur_range_info) */ 
-  owner->key_buffer->setup_reading(&cur_index_tuple, owner->keypar.key_size_in_keybuf,
-                            owner->is_mrr_assoc? (uchar**)&owner->cur_range_info: NULL,
-                            sizeof(void*));
+  owner->key_buffer->setup_reading(&cur_index_tuple, 
+                                   owner->keypar.key_size_in_keybuf,
+                                   owner->is_mrr_assoc? 
+                                     (uchar**)&owner->cur_range_info: NULL,
+                                   sizeof(void*));
 
   if (identical_key_it.read())
     return TRUE;
@@ -1106,16 +1115,18 @@
   uchar *save_cur_index_tuple= cur_index_tuple;
   while (!identical_key_it.read())
   {
-    if (Mrr_ordered_index_reader::key_tuple_cmp(owner, key_in_buf, cur_index_tuple))
+    if (owner->skip_identical_key_handling &&
+        Mrr_ordered_index_reader::key_tuple_cmp(owner, key_in_buf, 
+                                                cur_index_tuple))
       break;
     last_identical_key_ptr= cur_index_tuple;
   }
   identical_key_it.init(owner->key_buffer);
   cur_index_tuple= save_cur_index_tuple;
   res= owner->h->ha_index_read_map(owner->h->get_table()->record[0], 
-                            cur_index_tuple, 
-                            owner->keypar.key_tuple_map, 
-                            HA_READ_KEY_EXACT);
+                                   cur_index_tuple, 
+                                   owner->keypar.key_tuple_map, 
+                                   HA_READ_KEY_EXACT);
 
   if (res)
   {
@@ -1156,6 +1167,7 @@
   return 0;
 }
 
+
 void Key_value_records_iterator::close()
 {
   while (!owner->key_buffer->read() && 
@@ -1178,7 +1190,6 @@
       break; /* EOF or error */
   }
   return res;
-  //return strategy->get_next(range_info);
 }
 
 

=== modified file 'sql/multi_range_read.h'
--- a/sql/multi_range_read.h	2010-11-02 21:09:28 +0000
+++ b/sql/multi_range_read.h	2010-11-07 15:21:27 +0000
@@ -77,25 +77,60 @@
 /**
   A class to enumerate (record, range_id) pairs that match given key value.
   
-  The idea is that we have an array of
-
-    (key, range_id1), (key, range_id2) ... (key, range_idN)
-
-  pairs, i.e. multiple identical key values with their different range_id-s,
-  and also we have ha_engine object where we can find matches for the key
-  value.
-
-  What this class does is produces all combinations of (key_match_record_X,
-  range_idN) pairs.
+  @note
+
+  The idea is that we have a Lifo_buffer which holds (key, range_id) pairs
+  ordered by key value. From the front of the buffer we see
+
+    (key_val1, range_id1), (key_val1, range_id2) ... (key_val2, range_idN)
+
+  we take the first elements that have the same key value (key_val1 in the
+  example above), and make lookup into the table.  The table will have 
+  multiple matches for key_val1:
+ 
+                  == Table Index ==
+                   ...
+     key_val1 ->  key_val1, index_tuple1
+                  key_val1, index_tuple2
+                   ...
+                  key_val1, index_tupleN
+                   ...
+  
+  Our goal is to produce all possible combinations, i.e. we need:
+  
+    {(key_val1, index_tuple1), range_id1}
+    {(key_val1, index_tuple1), range_id2}
+       ...           ...               |
+    {(key_val1, index_tuple1), range_idN},
+                  
+    {(key_val1, index_tuple2), range_id1}
+    {(key_val1, index_tuple2), range_id2}
+        ...          ...               |
+    {(key_val1, index_tuple2), range_idN},
+
+        ...          ...          ...                          
+
+    {(key_val1, index_tupleK), range_idN}
 */
 
 class Key_value_records_iterator
 {
   /* Use this to get table handler, key buffer and other parameters */
   Mrr_ordered_index_reader *owner;
+
+  /* Iterator to get (key, range_id) pairs from */
   Lifo_buffer_iterator identical_key_it;
-
+  
+  /* 
+    Last of the identical key values (when we get this pointer from
+    identical_key_it, it will be time to stop).
+  */
   uchar *last_identical_key_ptr;
+
+  /*
+    FALSE <=> we're right after the init() call, the record has been already
+    read with owner->h->index_read_map() call
+  */
   bool get_next_row;
   
   uchar *cur_index_tuple; /* key_buffer.read() reads to here */
@@ -113,10 +148,23 @@
 class Buffer_manager
 {
 public:
+  /* 
+    Index-based reader calls this when it gets the first key, so we get to know
+    key length and 
+  */
   virtual void setup_buffer_sizes(uint key_size_in_keybuf, 
-                                  key_part_map key_tuple_map)=0;
-  virtual void reset_buffer_sizes()= 0;
-  virtual Lifo_buffer* get_key_buffer()= 0;
+                                  key_part_map key_tuple_map) = 0;
+
+  virtual void redistribute_buffer_space() = 0;
+  /* 
+    This is called when both key and rowid buffers are empty, and so it's time 
+    to reset them to their original size (They've lost their original size,
+    because we were dynamically growing rowid buffer and shrinking key buffer).
+  */
+  virtual void reset_buffer_sizes() = 0;
+
+  virtual Lifo_buffer* get_key_buffer() = 0;
+
   virtual ~Buffer_manager(){} /* Shut up the compiler */
 };
 
@@ -160,21 +208,21 @@
                    uint mode, Buffer_manager *buf_manager_arg) = 0;
 
   /* Get pointer to place where every get_next() call will put rowid */
-  virtual uchar *get_rowid_ptr()= 0;
+  virtual uchar *get_rowid_ptr() = 0;
   /* Get the rowid (call this after get_next() call) */
   void position();
-  virtual bool skip_record(char *range_id, uchar *rowid)=0;
+  virtual bool skip_record(char *range_id, uchar *rowid) = 0;
 };
 
 
 /*
-  A "bypass" reader that uses default MRR implementation (i.e.
-  handler::multi_range_read_XXX() calls) to produce rows.
+  A "bypass" index reader that just does and index scan. The index scan is done 
+  by calling default MRR implementation (i.e.  handler::multi_range_read_XXX())
+  functions.
 */
 
 class Mrr_simple_index_reader : public Mrr_index_reader
 {
-  int res; 
 public:
   int init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, 
            void *seq_init_param, uint n_ranges,
@@ -209,6 +257,13 @@
     return (mrr_funcs.skip_record &&
             mrr_funcs.skip_record(mrr_iter, range_info, rowid));
   }
+
+  bool skip_index_tuple(char *range_info)
+  {
+    return (mrr_funcs.skip_index_tuple &&
+            mrr_funcs.skip_index_tuple(mrr_iter, range_info));
+  }
+
 private:
   Key_value_records_iterator kv_it;
 
@@ -218,22 +273,28 @@
 
   /* Buffer to store (key, range_id) pairs */
   Lifo_buffer *key_buffer;
-
+  
+  /* Ask this to allocat*/
   Buffer_manager *buf_manager;
 
-  /* Initially FALSE, becomes TRUE when we've set key_tuple_xxx members */
+  /* 
+    Initially FALSE, becomes TRUE when we saw the first lookup key and set 
+    keypar's member.
+  */
   bool know_key_tuple_params;
-
-  Key_parameters  keypar;
+ 
+  Key_parameters  keypar; /* index scan and lookup tuple parameters */
 
   /* TRUE <=> need range association, buffers hold {rowid, range_id} pairs */
   bool is_mrr_assoc;
-
+  
+  /* TRUE <=> Don't do psergey-todo... */
+  bool skip_identical_key_handling;
+  
+  /* Range sequence iteration members */
   RANGE_SEQ_IF mrr_funcs;
   range_seq_t mrr_iter;
 
-  bool index_scan_eof;
-
   static int key_tuple_cmp(void* arg, uchar* key1, uchar* key2);
   static int key_tuple_cmp_reverse(void* arg, uchar* key1, uchar* key2);
   
@@ -256,18 +317,27 @@
   int get_next(char **range_info);
   int refill_buffer();
 private:
-  handler *h;
+  handler *h; /* Handler to use */
   
-  DsMrr_impl *dsmrr;
   /* This what we get (rowid, range_info) pairs from */
   Mrr_index_reader *index_reader;
+
+  /* index_reader->get_next() puts rowid here */
   uchar *index_rowid;
+  
+  /* TRUE <=> index_reader->refill_buffer() call has returned EOF */
   bool index_reader_exhausted;
   
   /* TRUE <=> need range association, buffers hold {rowid, range_id} pairs */
   bool is_mrr_assoc;
-
+  
+  /* 
+    When reading from ordered rowid buffer: the rowid element of the last
+    buffer element that has rowid identical to this one.
+  */
   uchar *last_identical_rowid;
+
+  /* Buffer to store (rowid, range_id) pairs */
   Lifo_buffer *rowid_buffer;
   
   /* rowid_buffer.read() will set the following:  */
@@ -278,7 +348,11 @@
 };
 
 
-/* A place where one can get readers without having to alloc them on the heap */
+/*
+  A primitive "factory" of various Mrr_*_reader classes (the point is to 
+  get various kinds of readers without having to allocate them on the heap)
+*/
+
 class Mrr_reader_factory
 {
 public:
@@ -497,10 +571,9 @@
                                uint *buffer_size, COST_VECT *cost);
   bool check_cpk_scan(THD *thd, uint keyno, uint mrr_flags);
 
-  void reallocate_buffer_space();
-  
   /* Buffer_manager implementation */
   void setup_buffer_sizes(uint key_size_in_keybuf, key_part_map key_tuple_map);
+  void redistribute_buffer_space();
   void reset_buffer_sizes();
   Lifo_buffer* get_key_buffer() { return key_buffer; }
 



More information about the commits mailing list