Cleanup's during review

Added ASSERT() to detect wrongly packed fields

sql/field.h

@@ -439,10 +439,12 @@ class Field_new_decimal :public Field_num {
   /* The maximum number of decimal digits can be stored */
   uint precision;
   uint bin_size;
-  /* Constructors take max_length of the field as a parameter - not the */
-  /* precision as the number of decimal digits allowed                  */
-  /* So for example we need to count length from precision handling     */
-  /* CREATE TABLE ( DECIMAL(x,y))                                       */
+  /*
+    Constructors take max_length of the field as a parameter - not the
+    precision as the number of decimal digits allowed.
+    So for example we need to count length from precision handling
+    CREATE TABLE ( DECIMAL(x,y)) 
+  */
   Field_new_decimal(char *ptr_arg, uint32 len_arg, uchar *null_ptr_arg,
                     uchar null_bit_arg,
                     enum utype unireg_check_arg, const char *field_name_arg,

sql/opt_range.cc

@@ -3307,32 +3307,35 @@ QUICK_SELECT_I *TRP_ROR_UNION::make_quick(PARAM *param,
 
 
 /*
-  Build a SEL_TREE for <> predicate
+  Build a SEL_TREE for <> or NOT BETWEEN predicate
  
   SYNOPSIS
     get_ne_mm_tree()
       param       PARAM from SQL_SELECT::test_quick_select
       cond_func   item for the predicate
       field       field in the predicate
-      value       constant in the predicate
+      lt_value    constant that field should be smaller
+      gt_value    constant that field should be greaterr
       cmp_type    compare type for the field
 
   RETURN 
-    Pointer to tree built tree
+    #  Pointer to tree built tree
+    0  on error
 */
 
 static SEL_TREE *get_ne_mm_tree(PARAM *param, Item_func *cond_func, 
-                                Field *field, Item *value,
+                                Field *field,
+                                Item *lt_value, Item *gt_value,
                                 Item_result cmp_type)
 {
-  SEL_TREE *tree= 0;
+  SEL_TREE *tree;
   tree= get_mm_parts(param, cond_func, field, Item_func::LT_FUNC,
-		       value, cmp_type);
+                     lt_value, cmp_type);
   if (tree)
   {
     tree= tree_or(param, tree, get_mm_parts(param, cond_func, field,
 					    Item_func::GT_FUNC,
-					    value, cmp_type));
+					    gt_value, cmp_type));
   }
   return tree;
 }
@@ -3365,21 +3368,14 @@ static SEL_TREE *get_func_mm_tree(PARAM *param, Item_func *cond_func,
   switch (cond_func->functype()) {
 
   case Item_func::NE_FUNC:
-    tree= get_ne_mm_tree(param, cond_func, field, value, cmp_type);
+    tree= get_ne_mm_tree(param, cond_func, field, value, value, cmp_type);
     break;
 
   case Item_func::BETWEEN:
     if (inv)
     {
-      tree= get_mm_parts(param, cond_func, field, Item_func::LT_FUNC,
-		         cond_func->arguments()[1],cmp_type);
-      if (tree)
-      {
-        tree= tree_or(param, tree, get_mm_parts(param, cond_func, field,
-					        Item_func::GT_FUNC,
-					        cond_func->arguments()[2],
-                                                cmp_type));
-      }
+      tree= get_ne_mm_tree(param, cond_func, field, cond_func->arguments()[1],
+                           cond_func->arguments()[2], cmp_type);
     }
     else
     {
@@ -3402,7 +3398,8 @@ static SEL_TREE *get_func_mm_tree(PARAM *param, Item_func *cond_func,
     if (inv)
     {
       tree= get_ne_mm_tree(param, cond_func, field,
-                           func->arguments()[1], cmp_type);
+                           func->arguments()[1], func->arguments()[1],
+                           cmp_type);
       if (tree)
       {
         Item **arg, **end;
@@ -3410,7 +3407,7 @@ static SEL_TREE *get_func_mm_tree(PARAM *param, Item_func *cond_func,
              arg < end ; arg++)
         {
           tree=  tree_and(param, tree, get_ne_mm_tree(param, cond_func, field, 
-                                                      *arg, cmp_type));
+                                                      *arg, *arg, cmp_type));
         }
       }
     }
@@ -3523,17 +3520,18 @@ static SEL_TREE *get_mm_tree(PARAM *param,COND *cond)
   Item_func *cond_func= (Item_func*) cond;
   if (cond_func->functype() == Item_func::NOT_FUNC)
   {
+    /* Optimize NOT BETWEEN and NOT IN */
     Item *arg= cond_func->arguments()[0];
     if (arg->type() == Item::FUNC_ITEM)
     {
       cond_func= (Item_func*) arg;
       if (cond_func->select_optimize() == Item_func::OPTIMIZE_NONE)
         DBUG_RETURN(0);
-      inv= TRUE;	
+      inv= TRUE;
     }
     else
       DBUG_RETURN(0);
-  }    
+  }
   else if (cond_func->select_optimize() == Item_func::OPTIMIZE_NONE)
     DBUG_RETURN(0);			       
 

sql/sql_base.cc

@@ -1607,9 +1607,9 @@ static int open_unireg_entry(THD *thd, TABLE *entry, const char *db,
         trying to discover the table at the same time.
       */
       if (discover_retry_count++ != 0)
-       goto err;
+        goto err;
       if (ha_create_table_from_engine(thd, db, name, TRUE) != 0)
-       goto err;
+        goto err;
 
       mysql_reset_errors(thd, 1);    // Clear warnings
       thd->clear_error();            // Clear error message

sql/sql_insert.cc

@@ -872,9 +872,10 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
       if (info->handle_duplicates == DUP_UPDATE)
       {
         int res= 0;
-        /* we don't check for other UNIQUE keys - the first row
-           that matches, is updated. If update causes a conflict again,
-           an error is returned
+        /*
+          We don't check for other UNIQUE keys - the first row
+          that matches, is updated. If update causes a conflict again,
+          an error is returned
         */
 	DBUG_ASSERT(table->insert_values != NULL);
         store_record(table,insert_values);

sql/sql_select.cc

@@ -2825,17 +2825,14 @@ add_key_fields(KEY_FIELD **key_fields,uint *and_level,
   if (cond_func->functype() == Item_func::NOT_FUNC)
   {
     Item *item= cond_func->arguments()[0];
-    /* 
-       At this moment all NOT before simple comparison predicates
-       are eliminated. NOT IN and NOT BETWEEN are treated similar
-       IN and BETWEEN respectively.
+    /*
+      At this moment all NOT before simple comparison predicates
+      are eliminated. NOT IN and NOT BETWEEN are treated similar
+      IN and BETWEEN respectively.
     */
     if (item->type() == Item::FUNC_ITEM &&
         ((Item_func *) item)->select_optimize() == Item_func::OPTIMIZE_KEY)
-    {
       add_key_fields(key_fields,and_level,item,usable_tables);
-      return;
-    }
     return;
   }
   switch (cond_func->select_optimize()) {

sql/unireg.cc

@@ -73,6 +73,7 @@ bool mysql_create_frm(THD *thd, my_string file_name,
 		      handler *db_file)
 {
   uint reclength,info_length,screens,key_info_length,maxlength;
+  ulong key_buff_length;
   File file;
   ulong filepos, data_offset;
   uchar fileinfo[64],forminfo[288],*keybuff;
@@ -119,7 +120,7 @@ bool mysql_create_frm(THD *thd, my_string file_name,
     DBUG_RETURN(1);
   }
 
-  uint key_buff_length=keys*(7+NAME_LEN+MAX_REF_PARTS*9)+16;
+  key_buff_length= uint4korr(fileinfo+47);
   keybuff=(uchar*) my_malloc(key_buff_length, MYF(0));
   key_info_length= pack_keys(keybuff, keys, key_info, data_offset);
   VOID(get_form_pos(file,fileinfo,&formnames));
@@ -128,7 +129,6 @@ bool mysql_create_frm(THD *thd, my_string file_name,
   maxlength=(uint) next_io_size((ulong) (uint2korr(forminfo)+1000));
   int2store(forminfo+2,maxlength);
   int4store(fileinfo+10,(ulong) (filepos+maxlength));
-  int4store(fileinfo+47,key_buff_length);
   fileinfo[26]= (uchar) test((create_info->max_rows == 1) &&
 			     (create_info->min_rows == 1) && (keys == 0));
   int2store(fileinfo+28,key_info_length);
@@ -411,7 +411,8 @@ static bool pack_header(uchar *forminfo, enum db_type table_type,
     DBUG_RETURN(1);
   }
 
-  totlength=reclength=0L;
+  totlength= 0L;
+  reclength= data_offset;
   no_empty=int_count=int_parts=int_length=time_stamp_pos=null_fields=
     com_length=0;
   n_length=2L;
@@ -440,6 +441,8 @@ static bool pack_header(uchar *forminfo, enum db_type table_type,
 	!time_stamp_pos)
       time_stamp_pos= (uint) field->offset+ (uint) data_offset + 1;
     length=field->pack_length;
+    /* Ensure we don't have any bugs when generating offsets */
+    DBUG_ASSERT(reclength == field->offset + data_offset);
     if ((uint) field->offset+ (uint) data_offset+ length > reclength)
       reclength=(uint) (field->offset+ data_offset + length);
     n_length+= (ulong) strlen(field->field_name)+1;