bzr commit into mysql-5.1 branch (mattias.jonsson:2751) Bug#37721

[Mattias Jonsson]

#At file:///Users/mattiasj/clones/bzrroot/b37721-51-bugteam/

 2751 Mattias Jonsson	2008-10-03
      Bug#37721: ORDER BY when WHERE contains non-partitioned
      index column
      
      There was actually two problems
      1) when clustered pk, order by non pk index should also
      compare with pk as last resort to differ keys from each
      other
      2) bug in the index search handling in ha_partition (was
      found when extending the test case
      
      Solution to 1 was to include the pk in key compare if
      clustered pk and search on other index.
      
      2 was just a faulty compare of key_length.
modified:
  mysql-test/r/partition_innodb.result
  mysql-test/t/partition_innodb.test
  sql/ha_partition.cc
  sql/ha_partition.h
  sql/key.cc

per-file messages:
  mysql-test/r/partition_innodb.result
    Bug#37721: ORDER BY when WHERE contains non-partitioned
    index column
    
    updated test result
  mysql-test/t/partition_innodb.test
    Bug#37721: ORDER BY when WHERE contains non-partitioned
    index column
    
    Added testcase for bug verification
  sql/ha_partition.cc
    Bug#37721: ORDER BY when WHERE contains non-partitioned
    index column
    
    using m_curr_key_info with both given index and PK
    if clustered PK.
    Also including PK in read_set
    Added debug prints for easier verification
  sql/ha_partition.h
    Bug#37721: ORDER BY when WHERE contains non-partitioned
    index column
    
    Changed m_curr_key_info to a null terminated array
    with max 2 keys and a terminating null.
    For use with key_rec_cmp with both given index and PK
  sql/key.cc
    Bug#37721: ORDER BY when WHERE contains non-partitioned
    index column
    
    added handling of a null terminated array of keys for
    use in compare.
    
    removed old row in synopsis
=== modified file 'mysql-test/r/partition_innodb.result'
--- a/mysql-test/r/partition_innodb.result	2008-03-07 21:46:29 +0000
+++ b/mysql-test/r/partition_innodb.result	2008-10-03 18:26:04 +0000
@@ -1,3 +1,68 @@
+# Bug#37721, test of ORDER BY on PK and WHERE on INDEX
+CREATE TABLE t1 (
+a INT,
+b INT,
+PRIMARY KEY (a),
+INDEX (b))
+ENGINE InnoDB
+PARTITION BY HASH(a)
+PARTITIONS 3;
+INSERT INTO t1 VALUES (0,0),(4,0),(2,0);
+SELECT a FROM t1 WHERE b = 0 ORDER BY a ASC;
+a
+0
+2
+4
+SELECT a FROM t1 WHERE b = 0 ORDER BY a DESC;
+a
+4
+2
+0
+ALTER TABLE t1 DROP INDEX b;
+SELECT a FROM t1 WHERE b = 0 ORDER BY a ASC;
+a
+0
+2
+4
+SELECT a FROM t1 WHERE b = 0 ORDER BY a DESC;
+a
+4
+2
+0
+DROP TABLE t1;
+CREATE TABLE t1 (
+a VARCHAR(600),
+b VARCHAR(600),
+PRIMARY KEY (a),
+INDEX (b))
+ENGINE InnoDB
+PARTITION BY KEY(a)
+PARTITIONS 3;
+INSERT INTO t1 VALUES (concat(repeat('MySQL',100),'1'),repeat('0',257));
+INSERT INTO t1 VALUES (concat(repeat('MySQL',100),'3'),repeat('0',257));
+INSERT INTO t1 VALUES (concat(repeat('MySQL',100),'2'),repeat('0',257));
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a ASC;
+right(a,1)
+1
+2
+3
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a DESC;
+right(a,1)
+3
+2
+1
+ALTER TABLE t1 DROP INDEX b;
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a ASC;
+right(a,1)
+1
+2
+3
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a DESC;
+right(a,1)
+3
+2
+1
+DROP TABLE t1;
 # Bug#32948
 CREATE TABLE t1 (c1 INT, PRIMARY KEY (c1)) ENGINE=INNODB;
 CREATE TABLE t2 (c1 INT, PRIMARY KEY (c1),

=== modified file 'mysql-test/t/partition_innodb.test'
--- a/mysql-test/t/partition_innodb.test	2008-03-07 21:46:29 +0000
+++ b/mysql-test/t/partition_innodb.test	2008-10-03 18:26:04 +0000
@@ -1,6 +1,46 @@
 --source include/have_partition.inc
 --source include/have_innodb.inc
 
+#
+# Bug37721: ORDER BY when WHERE contains non-partitioned index column
+# wrong order since it did not use pk as second compare
+--echo # Bug#37721, test of ORDER BY on PK and WHERE on INDEX
+CREATE TABLE t1 (
+  a INT,
+  b INT,
+  PRIMARY KEY (a),
+  INDEX (b))
+ENGINE InnoDB
+PARTITION BY HASH(a)
+PARTITIONS 3;
+# This will give the middle partition the highest value
+INSERT INTO t1 VALUES (0,0),(4,0),(2,0);
+SELECT a FROM t1 WHERE b = 0 ORDER BY a ASC;
+SELECT a FROM t1 WHERE b = 0 ORDER BY a DESC;
+ALTER TABLE t1 DROP INDEX b;
+SELECT a FROM t1 WHERE b = 0 ORDER BY a ASC;
+SELECT a FROM t1 WHERE b = 0 ORDER BY a DESC;
+DROP TABLE t1;
+CREATE TABLE t1 (
+  a VARCHAR(600),
+  b VARCHAR(600),
+  PRIMARY KEY (a),
+  INDEX (b))
+ENGINE InnoDB
+PARTITION BY KEY(a)
+PARTITIONS 3;
+# This will give the middle partition the highest value
+INSERT INTO t1 VALUES (concat(repeat('MySQL',100),'1'),repeat('0',257));
+INSERT INTO t1 VALUES (concat(repeat('MySQL',100),'3'),repeat('0',257));
+INSERT INTO t1 VALUES (concat(repeat('MySQL',100),'2'),repeat('0',257));
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a ASC;
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a DESC;
+ALTER TABLE t1 DROP INDEX b;
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a ASC;
+SELECT right(a,1) FROM t1 WHERE b = repeat('0',257) ORDER BY a DESC;
+DROP TABLE t1;
+
+#
 # Bug#32948 - FKs allowed to reference partitioned table
 #
 -- echo # Bug#32948

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2008-08-20 15:29:14 +0000
+++ b/sql/ha_partition.cc	2008-10-03 18:26:04 +0000
@@ -237,7 +237,8 @@ void ha_partition::init_handler_variable
   m_rec_length= 0;
   m_last_part= 0;
   m_rec0= 0;
-  m_curr_key_info= 0;
+  m_curr_key_info[0]= NULL;
+  m_curr_key_info[1]= NULL;
   /*
     this allows blackhole to work properly
   */
@@ -3565,11 +3566,25 @@ int ha_partition::index_init(uint inx, b
   handler **file;
   DBUG_ENTER("ha_partition::index_init");
 
+  DBUG_PRINT("info", ("inx %u sorted %u", inx, sorted));
   active_index= inx;
   m_part_spec.start_part= NO_CURRENT_PART_ID;
   m_start_key.length= 0;
   m_ordered= sorted;
-  m_curr_key_info= table->key_info+inx;
+  m_curr_key_info[0]= table->key_info+inx;
+  if (table->s->primary_key != MAX_KEY &&
+      m_file[0]->primary_key_is_clustered())
+  {
+    /*
+      if PK is clustered, then the key cmp must use the pk to
+      differentiate between equal key in given index.
+    */
+    DBUG_PRINT("info", ("Clustered pk, using pk as secondary cmp"));
+    m_curr_key_info[1]= table->key_info+table->s->primary_key;
+    m_curr_key_info[2]= NULL;
+  }
+  else
+    m_curr_key_info[1]= NULL;
   /*
     Some handlers only read fields as specified by the bitmap for the
     read set. For partitioned handlers we always require that the
@@ -3594,9 +3609,13 @@ int ha_partition::index_init(uint inx, b
       TODO: handle COUNT(*) queries via unordered scan.
     */
     uint i;
-    for (i= 0; i < m_curr_key_info->key_parts; i++)
-      bitmap_set_bit(table->read_set,
-                     m_curr_key_info->key_part[i].field->field_index);
+    KEY **key_info= m_curr_key_info;
+    do
+    {
+      for (i= 0; i < (*key_info)->key_parts; i++)
+        bitmap_set_bit(table->read_set,
+                       (*key_info)->key_part[i].field->field_index);
+    } while (*(++key_info));
   }
   file= m_file;
   do
@@ -3704,6 +3723,8 @@ int ha_partition::common_index_read(ucha
   uint key_len= calculate_key_len(table, active_index, key, keypart_map);
   DBUG_ENTER("ha_partition::common_index_read");
 
+  DBUG_PRINT("info", ("m_ordered %u m_ordered_scan_ongoing %u find_flag %u",
+                      m_ordered, m_ordered_scan_ongoing, find_flag));
   memcpy((void*)m_start_key.key, key, key_len);
   m_start_key.keypart_map= keypart_map;
   m_start_key.length= key_len;
@@ -3720,9 +3741,12 @@ int ha_partition::common_index_read(ucha
     reverse_order= TRUE;
     m_ordered_scan_ongoing= TRUE;
   }
+  DBUG_PRINT("info", ("m_ordered %u m_ordered_scan_ongoing %u find_flag %u (%u)",
+                      m_ordered, m_ordered_scan_ongoing, find_flag,
+                      HA_READ_KEY_EXACT));
   if (!m_ordered_scan_ongoing ||
       (find_flag == HA_READ_KEY_EXACT &&
-       (key_len >= m_curr_key_info->key_length ||
+       (key_len > m_curr_key_info[0]->key_length ||
 	key_len == 0)))
   {
     /*
@@ -3736,6 +3760,8 @@ int ha_partition::common_index_read(ucha
       Need to set unordered scan ongoing since we can come here even when
       it isn't set.
     */
+    DBUG_PRINT("info", ("key_len %lu (%lu), doing unordered scan",
+                        key_len, m_curr_key_info[0]->key_length));
     m_ordered_scan_ongoing= FALSE;
     error= handle_unordered_scan_next_partition(buf);
   }
@@ -3990,7 +4016,7 @@ int ha_partition::read_range_first(const
       ((end_key->flag == HA_READ_BEFORE_KEY) ? 1 :
        (end_key->flag == HA_READ_AFTER_KEY) ? -1 : 0);
   }
-  range_key_part= m_curr_key_info->key_part;
+  range_key_part= m_curr_key_info[0]->key_part;
 
   if (!start_key)				// Read first record
   {

=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h	2008-08-11 18:06:08 +0000
+++ b/sql/ha_partition.h	2008-10-03 18:26:04 +0000
@@ -67,7 +67,14 @@ private:
                                         // index scan info
   Field **m_part_field_array;           // Part field array locally to save acc
   uchar *m_ordered_rec_buffer;           // Row and key buffer for ord. idx scan
-  KEY *m_curr_key_info;                 // Current index
+  /*
+    Current index.
+    When used in key_rec_cmp: If clustered pk, index compare
+    must compare pk if given index is same for two rows.
+    So normally m_curr_key_info[0]= current index and m_curr_key[1]= NULL,
+    and if clustered pk, [0]= current index, [1]= pk, [2]= NULL
+  */
+  KEY *m_curr_key_info[3];              // Current index
   uchar *m_rec0;                         // table->record[0]
   QUEUE m_queue;                        // Prio queue used by sorted read
   /*

=== modified file 'sql/key.cc'
--- a/sql/key.cc	2008-02-07 14:09:59 +0000
+++ b/sql/key.cc	2008-10-03 18:26:04 +0000
@@ -452,8 +452,7 @@ int key_cmp(KEY_PART_INFO *key_part, con
   Compare two records in index order
   SYNOPSIS
     key_rec_cmp()
-    key                         Index information
-    rec0                        Pointer to table->record[0]
+    key                         Null termiated array of index information
     first_rec                   Pointer to record compare with
     second_rec                  Pointer to record compare against first_rec
 
@@ -473,9 +472,10 @@ int key_cmp(KEY_PART_INFO *key_part, con
     and return the result of the comparison.
 */
 
-int key_rec_cmp(void *key, uchar *first_rec, uchar *second_rec)
+int key_rec_cmp(void *key_p, uchar *first_rec, uchar *second_rec)
 {
-  KEY *key_info= (KEY*)key;
+  KEY **key= (KEY**) key_p;
+  KEY *key_info= *(key++);
   uint key_parts= key_info->key_parts, i= 0;
   KEY_PART_INFO *key_part= key_info->key_part;
   uchar *rec0= key_part->field->ptr - key_part->offset;
@@ -526,6 +526,12 @@ int key_rec_cmp(void *key, uchar *first_
                            key_part->length);
 next_loop:
     key_part++;
-  } while (!result && ++i < key_parts);
+    if (++i >= key_parts && (key_info= *(key++)))
+    {
+      key_parts= key_info->key_parts;
+      i= 0;
+      key_part= key_info->key_part;
+    }
+  } while (!result && key_info);
   DBUG_RETURN(result);
 }