Change create_field->offset to store offset from start of fields, independent of null bits.

Count null_bits separately from field offsets and adjust them in case of primary key parts.
(Previously a CREATE TABLE with a lot of null fields that was part of a primary key caused MySQL to wrongly count the number of bytes needed to store null bits)
This is a more complete bug fix for #6236


mysql-test/r/alter_table.result:
  More test for bug #6236 (CREATE TABLE didn't properly count not null columns for primary keys)
mysql-test/t/alter_table.test:
  More test for bug #6236 (CREATE TABLE didn't properly count not null columns for primary keys)
sql/handler.h:
  Add counter for null fields
sql/sql_table.cc:
  Change create_field->offset to store offset from start of fields, independent of null bits.
  Count null_bits separately from field offsets and adjust them in case of primary key parts.
sql/unireg.cc:
  Change create_field->offset to store offset from start of fields, independent of null bits.
  Count null_bits separately from field offsets and adjust them in case of primary key parts.

mysql-test/r/alter_table.result

@@ -406,3 +406,19 @@ t2	CREATE TABLE `t2` (
   PRIMARY KEY  (`a`)
 ) TYPE=MRG_MyISAM UNION=(t1)
 drop table if exists t1, t2;
+create table t1 (a int, b int, c int, d int, e int, f int, g int, h int,i int, primary key (a,b,c,d,e,f,g,i,h)) engine=MyISAM;
+insert into t1 (a) values(1);
+show table status like 't1';
+Name	Type	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Create_options	Comment
+t1	MyISAM	Fixed	1	37	37	X	X	X	X	X	X	X	X	
+alter table t1 modify a int;
+show table status like 't1';
+Name	Type	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Create_options	Comment
+t1	MyISAM	Fixed	1	37	37	X	X	X	X	X	X	X	X	
+drop table t1;
+create table t1 (a int not null, b int not null, c int not null, d int not null, e int not null, f int not null, g int not null, h int not null,i int not null, primary key (a,b,c,d,e,f,g,i,h)) engine=MyISAM;
+insert into t1 (a) values(1);
+show table status like 't1';
+Name	Type	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Create_options	Comment
+t1	MyISAM	Fixed	1	37	37	X	X	X	X	X	X	X	X	
+drop table t1;

mysql-test/t/alter_table.test

@@ -267,3 +267,20 @@ flush tables;
 alter table t1 modify a varchar(10) not null;
 show create table t2;
 drop table if exists t1, t2;
+
+# The following is also part of bug #6236 (CREATE TABLE didn't properly count
+# not null columns for primary keys)
+
+create table t1 (a int, b int, c int, d int, e int, f int, g int, h int,i int, primary key (a,b,c,d,e,f,g,i,h)) engine=MyISAM;
+insert into t1 (a) values(1);
+--replace_column 7 X 8 X 9 X 10 X 11 X 12 X 13 X 14 X
+show table status like 't1';
+alter table t1 modify a int;
+--replace_column 7 X 8 X 9 X 10 X 11 X 12 X 13 X 14 X
+show table status like 't1';
+drop table t1;
+create table t1 (a int not null, b int not null, c int not null, d int not null, e int not null, f int not null, g int not null, h int not null,i int not null, primary key (a,b,c,d,e,f,g,i,h)) engine=MyISAM;
+insert into t1 (a) values(1);
+--replace_column 7 X 8 X 9 X 10 X 11 X 12 X 13 X 14 X
+show table status like 't1';
+drop table t1;

sql/handler.h

@@ -164,6 +164,7 @@ typedef struct st_ha_create_information
   SQL_LIST merge_list;
   enum db_type db_type;
   enum row_type row_type;
+  uint null_bits;                       /* NULL bits at start of record */
   uint options;				/* OR of HA_CREATE_ options */
   uint raid_type,raid_chunks;
   uint merge_insert_method;

sql/sql_table.cc

@@ -363,7 +363,7 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
   create_field	*sql_field,*dup_field;
   int		error= -1;
   uint		db_options,field,null_fields,blob_columns;
-  ulong		pos;
+  ulong		record_offset;
   KEY	*key_info,*key_info_buffer;
   KEY_PART_INFO *key_part_info;
   int		auto_increment=0;
@@ -418,10 +418,9 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
     }
     it2.rewind();
   }
-  /* If fixed row records, we need one bit to check for deleted rows */
-  if (!(db_options & HA_OPTION_PACK_RECORD))
-    null_fields++;
-  pos=(null_fields+7)/8;
+
+  /* record_offset will be increased with 'length-of-null-bits' later */
+  record_offset= 0;
 
   it.rewind();
   while ((sql_field=it++))
@@ -478,10 +477,10 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
     }
     if (!(sql_field->flags & NOT_NULL_FLAG))
       sql_field->pack_flag|=FIELDFLAG_MAYBE_NULL;
-    sql_field->offset= pos;
+    sql_field->offset= record_offset;
     if (MTYP_TYPENR(sql_field->unireg_check) == Field::NEXT_NUMBER)
       auto_increment++;
-    pos+=sql_field->pack_length;
+    record_offset+= sql_field->pack_length;
   }
   if (auto_increment > 1)
   {
@@ -578,11 +577,12 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
 			column->field_name);
 	DBUG_RETURN(-1);
       }
-      /* for fulltext keys keyseg length is 1 for blobs (it's ignored in
-         ft code anyway, and 0 (set to column width later) for char's.
-         it has to be correct col width for char's, as char data are not
-         prefixed with length (unlike blobs, where ft code takes data length
-         from a data prefix, ignoring column->length).
+      /*
+        for fulltext keys keyseg length is 1 for blobs (it's ignored in
+        ft code anyway, and 0 (set to column width later) for char's.
+        it has to be correct col width for char's, as char data are not
+        prefixed with length (unlike blobs, where ft code takes data length
+        from a data prefix, ignoring column->length).
       */
       if (key->type == Key::FULLTEXT)
         column->length=test(f_is_blob(sql_field->pack_flag));
@@ -609,6 +609,7 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
 	  /* Implicitly set primary key fields to NOT NULL for ISO conf. */
 	  sql_field->flags|= NOT_NULL_FLAG;
 	  sql_field->pack_flag&= ~FIELDFLAG_MAYBE_NULL;
+          null_fields--;
 	}
         else
           key_info->flags|= HA_NULL_PART_KEY;
@@ -765,6 +766,7 @@ int mysql_create_table(THD *thd,const char *db, const char *table_name,
   if (thd->sql_mode & MODE_NO_DIR_IN_CREATE)
     create_info->data_file_name= create_info->index_file_name= 0;
   create_info->table_options=db_options;
+  create_info->null_bits= null_fields;
 
   if (rea_create_table(path, create_info, fields, key_count,
 		       key_info_buffer))
@@ -1829,17 +1831,6 @@ int mysql_alter_table(THD *thd,char *new_db, char *new_name,
 	     cfield->pack_length <= key_part_length))
 	  key_part_length=0;			// Use whole field
       }
-      if (!(cfield->flags & NOT_NULL_FLAG))
-      {
-	if (key_type == Key::PRIMARY)
-	{
-	  /* Implicitly set primary key fields to NOT NULL for ISO conf. */
-	  cfield->flags|= NOT_NULL_FLAG;
-	  cfield->pack_flag&= ~FIELDFLAG_MAYBE_NULL;
-	}
-        else
-          key_info->flags|= HA_NULL_PART_KEY;
-      }
       key_parts.push_back(new key_part_spec(cfield->field_name,
 					    key_part_length));
     }

sql/unireg.cc

@@ -32,18 +32,21 @@
 
 static uchar * pack_screens(List<create_field> &create_fields,
 			    uint *info_length, uint *screens, bool small_file);
-static uint pack_keys(uchar *keybuff,uint key_count, KEY *key_info);
+static uint pack_keys(uchar *keybuff,uint key_count, KEY *key_info,
+                      ulong data_offset);
 static bool pack_header(uchar *forminfo, enum db_type table_type,
 			List<create_field> &create_fields,
 			uint info_length, uint screens, uint table_options,
-			handler *file);
+			ulong data_offset, handler *file);
 static uint get_interval_id(uint *int_count,List<create_field> &create_fields,
 			    create_field *last_field);
-static bool pack_fields(File file, List<create_field> &create_fields);
+static bool pack_fields(File file, List<create_field> &create_fields,
+                        ulong data_offset);
 static bool make_empty_rec(int file, enum db_type table_type,
 			   uint table_options,
 			   List<create_field> &create_fields,
-			   uint reclength,uint null_fields);
+			   uint reclength, uint null_fields,
+                           ulong data_offset);
 
 
 int rea_create_table(my_string file_name,
@@ -53,7 +56,7 @@ int rea_create_table(my_string file_name,
 {
   uint reclength,info_length,screens,key_info_length,maxlength,null_fields;
   File file;
-  ulong filepos;
+  ulong filepos, data_offset;
   uchar fileinfo[64],forminfo[288],*keybuff;
   TYPELIB formnames;
   uchar *screen_buff;
@@ -64,8 +67,15 @@ int rea_create_table(my_string file_name,
   if (!(screen_buff=pack_screens(create_fields,&info_length,&screens,0)))
     DBUG_RETURN(1);
   db_file=get_new_handler((TABLE*) 0, create_info->db_type);
+
+ /* If fixed row records, we need one bit to check for deleted rows */
+  if (!(create_info->table_options & HA_OPTION_PACK_RECORD))
+    create_info->null_bits++;
+  data_offset= (create_info->null_bits + 7) / 8;
+
   if (pack_header(forminfo, create_info->db_type,create_fields,info_length,
-		  screens, create_info->table_options, db_file))
+		  screens, create_info->table_options,
+                  data_offset, db_file))
   {
     NET *net=my_pthread_getspecific_ptr(NET*,THR_NET);
     my_free((gptr) screen_buff,MYF(0));
@@ -77,7 +87,7 @@ int rea_create_table(my_string file_name,
     if (!(screen_buff=pack_screens(create_fields,&info_length,&screens,1)))
       DBUG_RETURN(1);
     if (pack_header(forminfo, create_info->db_type, create_fields,info_length,
-		    screens, create_info->table_options, db_file))
+		    screens, create_info->table_options, data_offset, db_file))
     {
       my_free((gptr) screen_buff,MYF(0));
       DBUG_RETURN(1);
@@ -95,7 +105,7 @@ int rea_create_table(my_string file_name,
 
   uint key_buff_length=uint2korr(fileinfo+14);
   keybuff=(uchar*) my_alloca(key_buff_length);
-  key_info_length=pack_keys(keybuff,keys,key_info);
+  key_info_length= pack_keys(keybuff, keys, key_info, data_offset);
   VOID(get_form_pos(file,fileinfo,&formnames));
   if (!(filepos=make_new_entry(file,fileinfo,&formnames,"")))
     goto err;
@@ -117,13 +127,13 @@ int rea_create_table(my_string file_name,
 	       (ulong) uint2korr(fileinfo+6)+ (ulong) key_buff_length,
 	       MY_SEEK_SET,MYF(0)));
   if (make_empty_rec(file,create_info->db_type,create_info->table_options,
-		     create_fields,reclength,null_fields))
+		     create_fields,reclength, null_fields, data_offset))
     goto err;
 
   VOID(my_seek(file,filepos,MY_SEEK_SET,MYF(0)));
   if (my_write(file,(byte*) forminfo,288,MYF_RW) ||
       my_write(file,(byte*) screen_buff,info_length,MYF_RW) ||
-      pack_fields(file,create_fields))
+      pack_fields(file, create_fields, data_offset))
     goto err;
 
 #ifdef HAVE_CRYPTED_FRM
@@ -248,7 +258,8 @@ static uchar * pack_screens(List<create_field> &create_fields,
 
 	/* Pack keyinfo and keynames to keybuff for save in form-file. */
 
-static uint pack_keys(uchar *keybuff,uint key_count,KEY *keyinfo)
+static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
+                      ulong data_offset)
 {
   uint key_parts,length;
   uchar *pos, *keyname_pos, *key_alg_pos;
@@ -273,10 +284,13 @@ static uint pack_keys(uchar *keybuff,uint key_count,KEY *keyinfo)
 	 key_part++)
 
     {
-      DBUG_PRINT("loop",("field: %d  startpos: %ld  length: %ld",
-			 key_part->fieldnr,key_part->offset,key_part->length));
+      uint offset;
+      DBUG_PRINT("loop",("field: %d  startpos: %lu  length: %ld",
+			 key_part->fieldnr, key_part->offset + data_offset,
+                         key_part->length));
       int2store(pos,key_part->fieldnr+1+FIELD_NAME_USED);
-      int2store(pos+2,key_part->offset+1);
+      offset= (uint) (key_part->offset+data_offset+1);
+      int2store(pos+2, offset);
       pos[4]=0;					// Sort order
       int2store(pos+5,key_part->key_type);
       int2store(pos+7,key_part->length);
@@ -316,8 +330,8 @@ static uint pack_keys(uchar *keybuff,uint key_count,KEY *keyinfo)
 
 static bool pack_header(uchar *forminfo, enum db_type table_type,
 			List<create_field> &create_fields,
-			uint info_length, uint screens,uint table_options,
-			handler *file)
+                        uint info_length, uint screens, uint table_options,
+                        ulong data_offset, handler *file)
 {
   uint length,int_count,int_length,no_empty, int_parts,
     time_stamp_pos,null_fields;
@@ -351,10 +365,10 @@ static bool pack_header(uchar *forminfo, enum db_type table_type,
     if ((MTYP_TYPENR(field->unireg_check) == Field::TIMESTAMP_FIELD ||
 	 f_packtype(field->pack_flag) == (int) FIELD_TYPE_TIMESTAMP) &&
 	!time_stamp_pos)
-      time_stamp_pos=(int) field->offset+1;
+      time_stamp_pos= (uint) field->offset+ (uint) data_offset + 1;
     length=field->pack_length;
-    if ((int) field->offset+length > reclength)
-      reclength=(int) field->offset+length;
+    if ((uint) field->offset+ (uint) data_offset+ length > reclength)
+      reclength=(uint) (field->offset+ data_offset + length);
     n_length+= (ulong) strlen(field->field_name)+1;
     field->interval_id=0;
     if (field->interval)
@@ -440,7 +454,8 @@ static uint get_interval_id(uint *int_count,List<create_field> &create_fields,
 
 	/* Save fields, fieldnames and intervals */
 
-static bool pack_fields(File file,List<create_field> &create_fields)
+static bool pack_fields(File file, List<create_field> &create_fields,
+                        ulong data_offset)
 {
   reg2 uint i;
   uint int_count;
@@ -455,11 +470,13 @@ static bool pack_fields(File file,List<create_field> &create_fields)
   int_count=0;
   while ((field=it++))
   {
+    uint recpos;
     buff[0]= (uchar) field->row;
     buff[1]= (uchar) field->col;
     buff[2]= (uchar) field->sc_length;
     buff[3]= (uchar) field->length;
-    uint recpos=(uint) field->offset+1;
+    /* The +1 is here becasue the col offset in .frm file have offset 1 */
+    recpos= field->offset+1 + (uint) data_offset;
     int2store(buff+4,recpos);
     int2store(buff+6,field->pack_flag);
     int2store(buff+8,field->unireg_check);
@@ -519,11 +536,12 @@ static bool pack_fields(File file,List<create_field> &create_fields)
 static bool make_empty_rec(File file,enum db_type table_type,
 			   uint table_options,
 			   List<create_field> &create_fields,
-			   uint reclength, uint null_fields)
+			   uint reclength, uint null_fields,
+                           ulong data_offset)
 {
   int error;
   Field::utype type;
-  uint firstpos,null_count,null_length;
+  uint firstpos,null_count;
   uchar *buff,*null_pos;
   TABLE table;
   create_field *field;
@@ -547,17 +565,16 @@ static bool make_empty_rec(File file,enum db_type table_type,
   firstpos=reclength;
   null_count=0;
   if (!(table_options & HA_OPTION_PACK_RECORD))
-  {
-    null_fields++;				// Need one bit for delete mark
-    null_count++;
-  }
-  bfill(buff,(null_length=(null_fields+7)/8),255);
+    null_count++;			// Need one bit for delete mark
+  DBUG_ASSERT(data_offset == ((null_fields + null_count + 7) / 8));
+  bfill(buff, (uint) data_offset, 255);
   null_pos=buff;
 
   List_iterator<create_field> it(create_fields);
   while ((field=it++))
   {
-    Field *regfield=make_field((char*) buff+field->offset,field->length,
+    Field *regfield=make_field((char*) buff+field->offset + data_offset,
+                               field->length,
 			       field->flags & NOT_NULL_FLAG ? 0:
 			       null_pos+null_count/8,
 			       1 << (null_count & 7),
@@ -570,9 +587,9 @@ static bool make_empty_rec(File file,enum db_type table_type,
     if (!(field->flags & NOT_NULL_FLAG))
       null_count++;
 
-    if ((uint) field->offset < firstpos &&
+    if ((uint) (field->offset + data_offset) < firstpos &&
 	regfield->type() != FIELD_TYPE_NULL)
-      firstpos= field->offset;
+      firstpos= field->offset + data_offset;
 
     type= (Field::utype) MTYP_TYPENR(field->unireg_check);
 
@@ -596,8 +613,8 @@ static bool make_empty_rec(File file,enum db_type table_type,
   }
 
   /* Fill not used startpos */
-  bfill((byte*) buff+null_length,firstpos-null_length,255);
-  error=(int) my_write(file,(byte*) buff,(uint) reclength,MYF_RW);
+  bfill((byte*) buff+data_offset, firstpos- (uint) data_offset, 255);
+  error=(int) my_write(file,(byte*) buff, (uint) reclength,MYF_RW);
   my_free((gptr) buff,MYF(MY_FAE));
   delete handler;
   DBUG_RETURN(error);