bk commit into 5.0 tree (patg:1.1936) BUG#9925

[Patrick Galbraith]

Below is the list of changes that have just been committed into a local
5.0 repository of patg. When patg does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.1936 05/06/11 02:34:08 patg@stripped +2 -0
  WL# 2653, bug# 9925
  
  ha_federated.h:
    variable name change
  ha_federated.cc:
    Savepoint for work on bug #9925. Index code now works without any problems
    though there are some questions I have about why the range->flag value seems
    incorrect in several cases, which I detailed in comments. This is also work
    for Worklog #2653

  sql/ha_federated.h
    1.14 05/06/11 02:33:15 patg@stripped +2 -2
    variable name change

  sql/ha_federated.cc
    1.30 05/06/11 02:33:15 patg@stripped +149 -60
    Savepoint for work on bug #9925. Index code now works without any problems
    though there are some questions I have about why the range->flag value seems
    incorrect in several cases, which I detailed in comments. This is also work
    for Worklog #2653

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	patg
# Host:	radha.local
# Root:	/Users/patg/mysql-5.0

--- 1.29/sql/ha_federated.cc	Tue Jun  7 23:44:39 2005
+++ 1.30/sql/ha_federated.cc	Sat Jun 11 02:33:15 2005
@@ -795,14 +795,16 @@
 bool ha_federated::create_where_from_key(String *to, KEY *key_info,
                                          const key_range *range, int range_type)
 {
-  uint second_loop= 0;
-  KEY_PART_INFO *key_part;
   bool needs_quotes;
+  const byte *key= range->key;
+  uint loop_counter= 0;
+  uint key_length= range->length;
+  KEY_PART_INFO *key_part;
   String tmp;
-  const byte *key;
-  uint key_length;
 
   DBUG_ENTER("ha_federated::create_where_from_key");
+  DBUG_PRINT("info",
+             ("range->flag %d", range->flag));
   if (range == NULL)
   {
     if (to->append(" TRUE ", 5))
@@ -810,16 +812,16 @@
     DBUG_RETURN(0);
   }
 
-  key= range->key;
-  key_length= range->length;
-
-  for (key_part= key_info->key_part; (int) key_length > 0; key_part++)
+  for (key_part= key_info->key_part;
+       key_length > 0;
+       key+= key_part->store_length,
+       key_length-= key_part->store_length, key_part++)
   {
     Field *field= key_part->field;
     needs_quotes= field->needs_quotes();
-    uint length= key_part->length;
+    DBUG_DUMP("key, start of loop", (char *) key, key_length);
 
-    if (second_loop++ && to->append(" AND ", 5))
+    if (loop_counter++ && to->append(" AND ", 5))
       DBUG_RETURN(1);
     if (to->append('`') || to->append(field->field_name) || to->append("` ",
2))
       DBUG_RETURN(1);                           // Out of memory
@@ -830,24 +832,85 @@
       {
         if (to->append("IS NULL", 7))
           DBUG_RETURN(1);
-
-        DBUG_PRINT("info",
-                   ("NULL type %s", to->c_ptr_quick()));
-        key_length-= key_part->store_length;
-        key+= key_part->store_length - 1;
         continue;
       }
-      key_length--;
     }
 
-    DBUG_PRINT("info",
-               ("range->flag %d", range->flag));
     switch(range->flag) {
-      /* 
-        in all cases of my testing, records_in_range has the range->flag
-        set to HA_READ_KEY_EXACT when it should be HA_READ_KEY_OR_NEXT
+      /*
+        Issues
+        simple table, test, id int, name varchar, primary key id
+
+        local query:
+        select * from test where id <= 4;
+        records_in_range calls create_where_key once
+        min_key, NULL (not called)
+        max_key->flag HA_READ_KEY_AFTER (wrong!)
+        remote query:
+        EXPLAIN SELECT `id`,`name` FROM `test` WHERE `id` > 4
+        (added a fix to make it `id` <= 4)
+        then:
+        read_range_first calls create_where_from_key once,
+        the same way records_in_range did. Why is the range->flag
+        HA_READ_KEY_AFTER when it should be HA_READ_KEY_OR_PREV?
+
+        local query:
+        select * from test where id = 4;
+        index_read_idx calls create_where_key
+        range_type NO_RANGE
+        range->flag HA_READ_KEY_EXACT,
+        remote query:
+        SELECT `id`,`name` FROM `test` WHERE `id`  = 4
+
+        local query:
+        select * from test where id >= 4
+        records in range calls create_where_from_key once
+        min_key->flag HA_READ_KEY_EXACT
+        and doesn't call with max_key (max_key NULL)
+        remote query:
+        EXPLAIN SELECT `id`,`name` FROM `test` WHERE `id`  = 4
+        then:
+        read_range_first calls create_where_from_key twice with
+        min_key->flag of HA_READ_KEY_OR_NEXT
+        max_key NULL (not called)
+        remote query:
+        SELECT `id`,`name` FROM `test` WHERE `id`  >= 4
+
+        different table, name now primary key (varchar pk)
+
+        local query:
+        select * from test3 where name = 'Fourth'
+        records_in_range calls create_where_from_key twice with
+        min_key->flag HA_READ_KEY_EXACT
+        max_key->flag HA_READ_AFTER_KEY
+        remote query:
+        EXPLAIN SELECT `id`,`name` FROM `test3` WHERE (`name`  = 'fourth') AND ( `name` 
<= 'fourth')
+        then:
+        index_read_idx calls create_where_from_key only once
+        range->flag HA_READ_KEY_EXACT
+        remote query:
+        SELECT `id`,`name` FROM `test3` WHERE `name`  = 'fourth'
+
+        local query:
+        select * from test3 where name >= 'Fourth'
+        records_in_range calls crete_where_from_key once with
+        min_key->flag HA_READ_KEY_EXACT
+        max_key NULL, not called
+        remote query:
+        EXPLAIN SELECT `id`,`name` FROM `test3` WHERE `name`  = 'fourth'
+        then:
+        read_range_first calls create_where_from_key once:
+        min_key->flag HA_READ_KEY_OR_NEXT
+        max_key NULL
+        remote query:
+        SELECT `id`,`name` FROM `test3` WHERE `name`  >= 'fourth'
+
       */
     case(HA_READ_KEY_EXACT):
+      /*
+        do we need special logic in this case to deal with the cases where it
+        should be something else?
+      */
       if (to->append(" = "))
         DBUG_RETURN(1);
       break;
@@ -859,9 +922,9 @@
         if (to->append(" <= "))
           DBUG_RETURN(1);
       break;
-      /* 
-        This is an issue which seems to be problematic in that when it is the 
-        max range and should be HA_READ_KEY_OR_PREV, it is wrong  and is 
+      /*
+        This is an issue which seems to be problematic in that when it is the
+        max range and should be HA_READ_KEY_OR_PREV, it is wrong  and is
         HA_READ_AFTER_KEY!
       */
     case(HA_READ_AFTER_KEY):
@@ -893,7 +956,7 @@
       /* This is can be treated as a hex string */
       Field_bit *field= (Field_bit *) (key_part->field);
       char buff[64 + 2], *ptr;
-      byte *end= (byte*)(key)+length;
+      byte *end= (byte*)(key)+key_part->store_length;
 
       buff[0]= '0';
       buff[1]= 'x';
@@ -905,27 +968,20 @@
       }
       if (to->append(buff, (uint)(ptr - buff)))
         DBUG_RETURN(1);
-
-      key_length-= length;
       continue;
     }
     if (key_part->key_part_flag & HA_BLOB_PART)
     {
       uint blob_length= uint2korr(key);
-      key+= HA_KEY_BLOB_LENGTH;
-      key_length-= HA_KEY_BLOB_LENGTH;
-
-      tmp.set_quick((char*) key, blob_length, &my_charset_bin);
+      tmp.set_quick((char*) key+HA_KEY_BLOB_LENGTH, blob_length, &my_charset_bin);
       if (append_escaped(to, &tmp))
         DBUG_RETURN(1);
 
-      length= key_part->length;
     }
     else if (key_part->key_part_flag & HA_VAR_LENGTH_PART)
     {
-      length= uint2korr(key);
-      key+= HA_KEY_BLOB_LENGTH;
-      tmp.set_quick((char*) key, length, &my_charset_bin);
+      uint var_length= uint2korr(key);
+      tmp.set_quick((char*) key+HA_KEY_BLOB_LENGTH, var_length, &my_charset_bin);
       if (append_escaped(to, &tmp))
         DBUG_RETURN(1);
     }
@@ -943,13 +999,13 @@
       }
       else if (to->append(res->ptr(), res->length()))
         DBUG_RETURN(1);
+
     }
     if (needs_quotes && to->append("'"))
       DBUG_RETURN(1);
+
     DBUG_PRINT("info",
                ("create_where_from_key WHERE clause: %s", to->c_ptr_quick()));
-    key+= length;
-    key_length-= length;
   }
   DBUG_RETURN(0);
 }
@@ -1091,23 +1147,40 @@
   USE Explain to get row estimate. Do not use count.
 
 */
+  bool both_not_null;
   char sql_query_buf[IO_SIZE];
-  MYSQL_RES *result=0;
-  MYSQL_ROW row;
   int error=0;
   ha_rows return_rows=20;
   String sql_query(sql_query_buf, sizeof(sql_query_buf), &my_charset_bin);
+  MYSQL_RES *result=0;
+  MYSQL_ROW row;
   DBUG_ENTER("ha_federated::records_in_range");
+  if (min_key == NULL && max_key == NULL)
+    DBUG_RETURN(0);
+  both_not_null= (min_key != NULL && max_key != NULL) ? TRUE : FALSE;
 
   sql_query.length(0);
   sql_query.append("EXPLAIN ");
   sql_query.append(share->select_query);
   sql_query.append(" WHERE ");
-  sql_query.append('(');
-  create_where_from_key(&sql_query, &table->key_info[inx], min_key,
MIN_RANGE);
-  sql_query.append(") AND ( ");
-  create_where_from_key(&sql_query, &table->key_info[inx], max_key,
MAX_RANGE);
-  sql_query.append(')');
+
+  /* need to move following lines to it's own method */
+  if (both_not_null)
+    sql_query.append('(');
+
+  if (min_key != NULL)
+    create_where_from_key(&sql_query,
+                          &table->key_info[inx], min_key, MIN_RANGE);
+
+  if (both_not_null)
+    sql_query.append(") AND ( ");
+
+  if (max_key != NULL)
+    create_where_from_key(&sql_query,
+                          &table->key_info[inx], max_key, MAX_RANGE);
+
+  if (both_not_null)
+    sql_query.append(')');
 
   DBUG_PRINT("info",("inx %d sql_query %s", inx, sql_query.c_ptr_quick()));
   if (mysql_real_query(mysql, sql_query.ptr(), sql_query.length()))
@@ -1762,24 +1835,48 @@
   DBUG_RETURN(0);
 }
 
-int ha_federated::read_range_first(const key_range *start_key,
-                                           const key_range *end_key,
+/*
+
+  int read_range_first(const key_range *min_key,
+                               const key_range *max_key,
+                               bool eq_range, bool sorted);
+*/
+int ha_federated::read_range_first(const key_range *min_key,
+                                           const key_range *max_key,
                                           bool eq_range, bool sorted)
 {
-  int retval;
+  bool both_not_null;
   char sql_query_buffer[IO_SIZE];
+  int retval;
   String sql_query(sql_query_buffer, sizeof(sql_query_buffer), &my_charset_bin);
 
   DBUG_ENTER("ha_federated::read_range_first");
+  if (min_key == NULL && max_key == NULL)
+    DBUG_RETURN(0);
+  both_not_null= (min_key != NULL && max_key != NULL) ? TRUE : FALSE;
+
   sql_query.length(0);
 
   sql_query.append(share->select_query);
   sql_query.append(" WHERE ");
-  sql_query.append('(');
-  create_where_from_key(&sql_query, &table->key_info[active_index], start_key,
MIN_RANGE);
-  sql_query.append(") AND (");
-  create_where_from_key(&sql_query, &table->key_info[active_index], end_key,
MAX_RANGE);
-  sql_query.append(')');
+
+  /* need to move following lines to it's own method */
+  if (both_not_null)
+    sql_query.append('(');
+
+  if (min_key != NULL)
+    create_where_from_key(&sql_query,
+                          &table->key_info[active_index], min_key, MIN_RANGE);
+
+  if (both_not_null)
+    sql_query.append(") AND (");
+
+  if (max_key != NULL)
+    create_where_from_key(&sql_query,
+                          &table->key_info[active_index], max_key, MAX_RANGE);
+
+  if (both_not_null)
+    sql_query.append(')');
 
   DBUG_PRINT("info",("active_index %d sql_query %s", active_index,
sql_query.c_ptr_quick()));
   if (mysql_real_query(mysql, sql_query.ptr(), sql_query.length()))
@@ -2025,14 +2122,6 @@
   DBUG_RETURN(0);
 }
 
-/*
-
-  int read_range_first(const key_range *start_key,
-                               const key_range *end_key,
-                               bool eq_range, bool sorted);
-  int read_range_next();
-
-*/
 
 /*
   ::info() is used to return information to the optimizer.

--- 1.13/sql/ha_federated.h	Tue Jun  7 23:44:40 2005
+++ 1.14/sql/ha_federated.h	Sat Jun 11 02:33:15 2005
@@ -183,8 +183,8 @@
                      uint key_len, enum ha_rkey_function find_flag);
   int index_next(byte * buf);
   int index_end();
-  int read_range_first(const key_range *start_key,
-                               const key_range *end_key,
+  int read_range_first(const key_range *min_key,
+                               const key_range *max_key,
                                bool eq_range, bool sorted);
   int read_range_next();
   /*