List:Commits« Previous MessageNext Message »
From:Georgi Kodinov Date:July 23 2008 11:27am
Subject:bzr push into mysql-5.0 branch (kgeorge:2646 to 2647) Bug#37830
View as plain text  
 2647 Georgi Kodinov	2008-07-23
      Bug#37830 : ORDER BY ASC/DESC - no difference
                        
      Range scan in descending order for c <= <col> <= c type of
      ranges was ignoring the DESC flag.
      However some engines like InnoDB have the primary key parts 
      as a suffix for every secondary key.
      When such primary key suffix is used for ordering ignoring 
      the DESC is not valid.
      But we generally would like to do this because it's faster.
                  
      Fixed by performing only reverse scan if the primary key is used.
      Removed some dead code in the process.
modified:
  mysql-test/r/innodb_mysql.result
  mysql-test/t/innodb_mysql.test
  sql/opt_range.cc
  sql/opt_range.h
  sql/sql_select.cc

 2646 Kristofer Pettersson	2008-07-22 [merge]
      merge 5.0->5.0-bugteam
modified:
  configure.in
  include/my_sys.h
  mysys/default.c
  mysys/mf_pack.c
  scripts/make_binary_distribution.sh

=== modified file 'mysql-test/r/innodb_mysql.result'
--- a/mysql-test/r/innodb_mysql.result	2008-02-07 07:12:49 +0000
+++ b/mysql-test/r/innodb_mysql.result	2008-07-23 11:25:00 +0000
@@ -1246,4 +1246,19 @@ set global innodb_autoextend_increment=@
 set @my_innodb_commit_concurrency=@@global.innodb_commit_concurrency;
 set global innodb_commit_concurrency=0;
 set global innodb_commit_concurrency=@my_innodb_commit_concurrency;
+CREATE TABLE t1 (a int, b int, c int, PRIMARY KEY (a), KEY t1_b (b))
+ENGINE=InnoDB;
+INSERT INTO t1 (a,b,c) VALUES (1,1,1), (2,1,1), (3,1,1), (4,1,1);
+INSERT INTO t1 (a,b,c) SELECT a+4,b,c FROM t1;
+EXPLAIN SELECT a, b, c FROM t1 WHERE b = 1 ORDER BY a DESC LIMIT 5;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	range	t1_b	t1_b	5	NULL	4	Using where
+SELECT a, b, c FROM t1 WHERE b = 1 ORDER BY a DESC LIMIT 5;
+a	b	c
+8	1	1
+7	1	1
+6	1	1
+5	1	1
+4	1	1
+DROP TABLE t1;
 End of 5.0 tests

=== modified file 'mysql-test/t/innodb_mysql.test'
--- a/mysql-test/t/innodb_mysql.test	2008-02-07 07:12:49 +0000
+++ b/mysql-test/t/innodb_mysql.test	2008-07-23 11:25:00 +0000
@@ -996,4 +996,22 @@ set @my_innodb_commit_concurrency=@@glob
 set global innodb_commit_concurrency=0;
 set global innodb_commit_concurrency=@my_innodb_commit_concurrency;
 
+#
+# Bug #37830: ORDER BY ASC/DESC - no difference
+#
+
+CREATE TABLE t1 (a int, b int, c int, PRIMARY KEY (a), KEY t1_b (b))
+ ENGINE=InnoDB;
+
+INSERT INTO t1 (a,b,c) VALUES (1,1,1), (2,1,1), (3,1,1), (4,1,1);
+INSERT INTO t1 (a,b,c) SELECT a+4,b,c FROM t1;
+
+# should be range access
+EXPLAIN SELECT a, b, c FROM t1 WHERE b = 1 ORDER BY a DESC LIMIT 5;
+
+# should produce '8 7 6 5 4' for a
+SELECT a, b, c FROM t1 WHERE b = 1 ORDER BY a DESC LIMIT 5;
+
+DROP TABLE t1;
+
 --echo End of 5.0 tests

=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2008-07-15 14:13:21 +0000
+++ b/sql/opt_range.cc	2008-07-23 11:25:00 +0000
@@ -7099,7 +7099,8 @@ bool QUICK_RANGE_SELECT::row_in_ranges()
 
 QUICK_SELECT_DESC::QUICK_SELECT_DESC(QUICK_RANGE_SELECT *q,
                                      uint used_key_parts_arg)
- : QUICK_RANGE_SELECT(*q), rev_it(rev_ranges)
+ : QUICK_RANGE_SELECT(*q), rev_it(rev_ranges), 
+  used_key_parts (used_key_parts_arg)
 {
   QUICK_RANGE *r;
 
@@ -7141,10 +7142,11 @@ int QUICK_SELECT_DESC::get_next()
     int result;
     if (last_range)
     {						// Already read through key
-      result = ((last_range->flag & EQ_RANGE)
-		? file->index_next_same(record, (byte*) last_range->min_key,
-					last_range->min_length) :
-		file->index_prev(record));
+      result = ((last_range->flag & EQ_RANGE && 
+                 used_key_parts <= head->key_info[index].key_parts) ? 
+                file->index_next_same(record, (byte*) last_range->min_key,
+                                      last_range->min_length) :
+                file->index_prev(record));
       if (!result)
       {
 	if (cmp_prev(*rev_it.ref()) == 0)
@@ -7168,7 +7170,9 @@ int QUICK_SELECT_DESC::get_next()
       continue;
     }
 
-    if (last_range->flag & EQ_RANGE)
+    if (last_range->flag & EQ_RANGE &&
+        used_key_parts <= head->key_info[index].key_parts)
+
     {
       result= file->index_read(record, (byte*) last_range->max_key,
                                last_range->max_length, HA_READ_KEY_EXACT);
@@ -7176,6 +7180,8 @@ int QUICK_SELECT_DESC::get_next()
     else
     {
       DBUG_ASSERT(last_range->flag & NEAR_MAX ||
+                  (last_range->flag & EQ_RANGE && 
+                   used_key_parts > head->key_info[index].key_parts) ||
                   range_reads_after_key(last_range));
       result=file->index_read(record, (byte*) last_range->max_key,
 			      last_range->max_length,
@@ -7273,54 +7279,6 @@ bool QUICK_SELECT_DESC::range_reads_afte
 }
 
 
-/* TRUE if we are reading over a key that may have a NULL value */
-
-#ifdef NOT_USED
-bool QUICK_SELECT_DESC::test_if_null_range(QUICK_RANGE *range_arg,
-					   uint used_key_parts)
-{
-  uint offset, end;
-  KEY_PART *key_part = key_parts,
-           *key_part_end= key_part+used_key_parts;
-
-  for (offset= 0,  end = min(range_arg->min_length, range_arg->max_length) ;
-       offset < end && key_part != key_part_end ;
-       offset+= key_part++->store_length)
-  {
-    if (!memcmp((char*) range_arg->min_key+offset,
-		(char*) range_arg->max_key+offset,
-		key_part->store_length))
-      continue;
-
-    if (key_part->null_bit && range_arg->min_key[offset])
-      return 1;				// min_key is null and max_key isn't
-    // Range doesn't cover NULL. This is ok if there is no more null parts
-    break;
-  }
-  /*
-    If the next min_range is > NULL, then we can use this, even if
-    it's a NULL key
-    Example:  SELECT * FROM t1 WHERE a = 2 AND b >0 ORDER BY a DESC,b DESC;
-
-  */
-  if (key_part != key_part_end && key_part->null_bit)
-  {
-    if (offset >= range_arg->min_length || range_arg->min_key[offset])
-      return 1;					// Could be null
-    key_part++;
-  }
-  /*
-    If any of the key parts used in the ORDER BY could be NULL, we can't
-    use the key to sort the data.
-  */
-  for (; key_part != key_part_end ; key_part++)
-    if (key_part->null_bit)
-      return 1;					// Covers null part
-  return 0;
-}
-#endif
-
-
 void QUICK_RANGE_SELECT::add_info_string(String *str)
 {
   KEY *key_info= head->key_info + index;

=== modified file 'sql/opt_range.h'
--- a/sql/opt_range.h	2007-02-21 12:07:08 +0000
+++ b/sql/opt_range.h	2008-07-23 11:25:00 +0000
@@ -667,12 +667,10 @@ public:
   int get_type() { return QS_TYPE_RANGE_DESC; }
 private:
   bool range_reads_after_key(QUICK_RANGE *range);
-#ifdef NOT_USED
-  bool test_if_null_range(QUICK_RANGE *range, uint used_key_parts);
-#endif
   int reset(void) { rev_it.rewind(); return QUICK_RANGE_SELECT::reset(); }
   List<QUICK_RANGE> rev_ranges;
   List_iterator<QUICK_RANGE> rev_it;
+  uint used_key_parts;
 };
 
 

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2008-07-15 14:13:21 +0000
+++ b/sql/sql_select.cc	2008-07-23 11:25:00 +0000
@@ -12088,26 +12088,25 @@ part_of_refkey(TABLE *table,Field *field
 }
 
 
-/*****************************************************************************
-  Test if one can use the key to resolve ORDER BY
-
-  SYNOPSIS
-    test_if_order_by_key()
-    order		Sort order
-    table		Table to sort
-    idx			Index to check
-    used_key_parts	Return value for used key parts.
-
-
-  NOTES
-    used_key_parts is set to correct key parts used if return value != 0
-    (On other cases, used_key_part may be changed)
+/**
+  Test if a key can be used to resolve ORDER BY
 
-  RETURN
-    1   key is ok.
-    0   Key can't be used
-    -1  Reverse key can be used
-*****************************************************************************/
+  used_key_parts is set to correct key parts used if return value != 0
+  (On other cases, used_key_part may be changed).
+  Note that the value may actually be greater than the number of index 
+  key parts. This can happen for storage engines that have the primary 
+  key parts as a suffix for every secondary key.
+
+  @param      order		Sort order
+  @param      table		Table to sort
+  @param      idx		Index to check
+  @param[out] used_key_parts	Return value for used key parts.
+
+  @return indication if the key can be used for sorting
+    @retval 1   key can be used for reading data in order.
+    @retval 0   Key can't be used
+    @retval -1  Reverse read on the key can be used
+*/
 
 static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
 				uint *used_key_parts)
@@ -12172,11 +12171,27 @@ static int test_if_order_by_key(ORDER *o
     reverse=flag;				// Remember if reverse
     key_part++;
   }
-  *used_key_parts= on_primary_key ? table->key_info[idx].key_parts :
-    (uint) (key_part - table->key_info[idx].key_part);
-  if (reverse == -1 && !(table->file->index_flags(idx, *used_key_parts-1, 1) &
-                         HA_READ_PREV))
-    reverse= 0;                                 // Index can't be used
+  if (on_primary_key)
+  {
+    uint used_key_parts_secondary= table->key_info[idx].key_parts;
+    uint used_key_parts_pk=
+      (uint) (key_part - table->key_info[table->s->primary_key].key_part);
+    *used_key_parts= used_key_parts_pk + used_key_parts_secondary;
+
+    if (reverse == -1 &&
+        (!(table->file->index_flags(idx, used_key_parts_secondary - 1, 1) &
+           HA_READ_PREV) ||
+         !(table->file->index_flags(table->s->primary_key,
+                                    used_key_parts_pk - 1, 1) & HA_READ_PREV)))
+      reverse= 0;                               // Index can't be used
+  }
+  else
+  {
+    *used_key_parts= (uint) (key_part - table->key_info[idx].key_part);
+    if (reverse == -1 && 
+        !(table->file->index_flags(idx, *used_key_parts-1, 1) & HA_READ_PREV))
+      reverse= 0;                               // Index can't be used
+  }
   DBUG_RETURN(reverse);
 }
 

Thread
bzr push into mysql-5.0 branch (kgeorge:2646 to 2647) Bug#37830Georgi Kodinov23 Jul