Fix for bug #5915 "ALTER TABLE behaves differently when converting column

to auto_increment in 4.1".
Now we are enforcing NO_AUTO_VALUE_ON_ZERO mode during ALTER TABLE only
if we are converting one auto_increment column to another auto_increment
column (this also includes most common case when we don't do anything
with such column).

Also now when we convert some column to TIMESTAMP NOT NULL column with
ALTER TABLE we convert NULL values to current timestamp, (as we do this
in INSERT). One can still get old behavior by setting system TIMESTAMP
variable to 0.

mysql-test/r/auto_increment.result

@@ -289,3 +289,55 @@ a	b
 0	13
 500	14
 drop table t1;
+create table t1 (a bigint);
+insert into t1 values (1), (2), (3), (NULL), (NULL);
+alter table t1 modify a bigint not null auto_increment primary key;
+select * from t1;
+a
+1
+2
+3
+4
+5
+drop table t1;
+create table t1 (a bigint);
+insert into t1 values (1), (2), (3), (0), (0);
+alter table t1 modify a bigint not null auto_increment primary key;
+select * from t1;
+a
+1
+2
+3
+4
+5
+drop table t1;
+create table t1 (a bigint);
+insert into t1 values (0), (1), (2), (3);
+set sql_mode=NO_AUTO_VALUE_ON_ZERO;
+alter table t1 modify a bigint not null auto_increment primary key;
+set sql_mode= '';
+select * from t1;
+a
+0
+1
+2
+3
+drop table t1;
+create table t1 (a int auto_increment primary key , b int null);
+set sql_mode=NO_AUTO_VALUE_ON_ZERO;
+insert into t1 values (0,1),(1,2),(2,3);
+select * from t1;
+a	b
+0	1
+1	2
+2	3
+set sql_mode= '';
+alter table t1 modify b varchar(255);
+insert into t1 values (0,4);
+select * from t1;
+a	b
+0	1
+1	2
+2	3
+3	4
+drop table t1;

mysql-test/r/type_timestamp.result

@@ -409,15 +409,12 @@ a	b
 NULL	NULL
 NULL	2003-01-01 00:00:00
 drop table t1;
-create table t1 (ts timestamp(19));
-show create table t1;
-Table	Create Table
-t1	CREATE TABLE `t1` (
-  `ts` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP
-) ENGINE=MyISAM DEFAULT CHARSET=latin1
-set TIMESTAMP=1000000000;
-insert into t1 values ();
+create table t1 (a bigint, b bigint);
+insert into t1 values (NULL, NULL), (20030101000000, 20030102000000);
+set timestamp=1000000019;
+alter table t1 modify a timestamp, modify b timestamp;
 select * from t1;
-ts
-2001-09-09 04:46:40
+a	b
+2001-09-09 04:46:59	2001-09-09 04:46:59
+2003-01-01 00:00:00	2003-01-02 00:00:00
 drop table t1;

mysql-test/t/auto_increment.test

@@ -168,3 +168,41 @@ update t1 set a=NULL where b=13;
 update t1 set a=500 where b=14;
 select * from t1 order by b;
 drop table t1;
+
+#
+# Test of behavior of ALTER TABLE when coulmn containing NULL or zeroes is
+# converted to AUTO_INCREMENT column
+#
+create table t1 (a bigint);
+insert into t1 values (1), (2), (3), (NULL), (NULL);
+alter table t1 modify a bigint not null auto_increment primary key; 
+select * from t1;
+drop table t1;
+
+create table t1 (a bigint);
+insert into t1 values (1), (2), (3), (0), (0);
+alter table t1 modify a bigint not null auto_increment primary key; 
+select * from t1;
+drop table t1;
+
+# We still should be able to preserve zero in NO_AUTO_VALUE_ON_ZERO mode
+create table t1 (a bigint);
+insert into t1 values (0), (1), (2), (3);
+set sql_mode=NO_AUTO_VALUE_ON_ZERO;
+alter table t1 modify a bigint not null auto_increment primary key; 
+set sql_mode= '';
+select * from t1;
+drop table t1;
+
+# It also sensible to preserve zeroes if we are converting auto_increment
+# column to auto_increment column (or not touching it at all, which is more
+# common case probably)
+create table t1 (a int auto_increment primary key , b int null);
+set sql_mode=NO_AUTO_VALUE_ON_ZERO;
+insert into t1 values (0,1),(1,2),(2,3);
+select * from t1;
+set sql_mode= '';
+alter table t1 modify b varchar(255);
+insert into t1 values (0,4);
+select * from t1;
+drop table t1;

mysql-test/t/type_timestamp.test

@@ -266,12 +266,12 @@ select * from t1;
 drop table t1;
 
 #
-# Test for bug #4491, TIMESTAMP(19) should be possible to create and not
-# only read in 4.0
+# Let us test behavior of ALTER TABLE when it converts columns 
+# containing NULL to TIMESTAMP columns.
 #
-create table t1 (ts timestamp(19));
-show create table t1;
-set TIMESTAMP=1000000000;
-insert into t1 values ();
+create table t1 (a bigint, b bigint);
+insert into t1 values (NULL, NULL), (20030101000000, 20030102000000);
+set timestamp=1000000019;
+alter table t1 modify a timestamp, modify b timestamp;
 select * from t1;
 drop table t1;

sql/field_conv.cc

@@ -252,7 +252,8 @@ static void do_copy_timestamp(Copy_field *copy)
 {
   if (*copy->from_null_ptr & copy->from_bit)
   {
-    ((Field_timestamp*) copy->to_field)->set_time();// Same as set_field_to_null
+    /* Same as in set_field_to_null_with_conversions() */
+    ((Field_timestamp*) copy->to_field)->set_time();
   }
   else
     (copy->do_copy2)(copy);
@@ -262,7 +263,11 @@ static void do_copy_timestamp(Copy_field *copy)
 static void do_copy_next_number(Copy_field *copy)
 {
   if (*copy->from_null_ptr & copy->from_bit)
-    copy->to_field->reset();			// Same as set_field_to_null
+  {
+    /* Same as in set_field_to_null_with_conversions() */
+    copy->to_field->table->auto_increment_field_not_null= FALSE;
+    copy->to_field->reset();
+  }
   else
     (copy->do_copy2)(copy);
 }
@@ -437,18 +442,20 @@ void Copy_field::set(Field *to,Field *from,bool save)
       }
     }
     else
-      do_copy=do_copy_not_null;
+    {
+      if (to_field->type() == FIELD_TYPE_TIMESTAMP)
+        do_copy= do_copy_timestamp;               // Automatic timestamp
+      else if (to_field == to_field->table->next_number_field)
+        do_copy= do_copy_next_number;
+      else
+        do_copy= do_copy_not_null;
+    }
   }
   else if (to_field->real_maybe_null())
   {
     to_null_ptr=	to->null_ptr;
     to_bit=		to->null_bit;
-    if (to_field->type() == FIELD_TYPE_TIMESTAMP)
-      do_copy=do_copy_timestamp;		// Automatic timestamp
-    else if (to_field == to_field->table->next_number_field)
-      do_copy=do_copy_next_number;
-    else
-      do_copy=do_copy_maybe_null;
+    do_copy= do_copy_maybe_null;
   }
   else
    do_copy=0;

sql/sql_table.cc

@@ -3300,13 +3300,14 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   List<Item>   all_fields;
   ha_rows examined_rows;
   bool auto_increment_field_copied= 0;
-  ulong old_sql_mode;
-  bool no_auto_on_zero;
+  ulong save_sql_mode;
   DBUG_ENTER("copy_data_between_tables");
 
   if (!(copy= new Copy_field[to->fields]))
     DBUG_RETURN(-1);				/* purecov: inspected */
 
+  save_sql_mode= thd->variables.sql_mode;
+
   to->file->external_lock(thd,F_WRLCK);
   from->file->info(HA_STATUS_VARIABLE);
   to->file->start_bulk_insert(from->file->records);
@@ -3320,7 +3321,17 @@ copy_data_between_tables(TABLE *from,TABLE *to,
     if (def->field)
     {
       if (*ptr == to->next_number_field)
+      {
         auto_increment_field_copied= TRUE;
+        /*
+          If we are going to copy contents of one auto_increment column to
+          another auto_increment column it is sensible to preserve zeroes.
+          This condition also covers case when we are don't actually alter
+          auto_increment column.
+        */
+        if (def->field == from->found_next_number_field)
+          thd->variables.sql_mode|= MODE_NO_AUTO_VALUE_ON_ZERO;
+      }
       (copy_end++)->set(*ptr,def->field,0);
     }
 
@@ -3360,11 +3371,6 @@ copy_data_between_tables(TABLE *from,TABLE *to,
     goto err;
   }
 
-  /* Turn on NO_AUTO_VALUE_ON_ZERO if not already on */
-  old_sql_mode= thd->variables.sql_mode;
-  if (!(no_auto_on_zero= thd->variables.sql_mode & MODE_NO_AUTO_VALUE_ON_ZERO))
-    thd->variables.sql_mode|= MODE_NO_AUTO_VALUE_ON_ZERO;
-
   /* Handler must be told explicitly to retrieve all columns, because
      this function does not set field->query_id in the columns to the
      current query id */
@@ -3422,10 +3428,6 @@ copy_data_between_tables(TABLE *from,TABLE *to,
 
   ha_enable_transaction(thd,TRUE);
 
-  /* Turn off NO_AUTO_VALUE_ON_ZERO if it was not already off */
-  if (!no_auto_on_zero)
-    thd->variables.sql_mode= old_sql_mode;
-
   /*
     Ensure that the new table is saved properly to disk so that we
     can do a rename
@@ -3437,6 +3439,7 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   if (to->file->external_lock(thd,F_UNLCK))
     error=1;
  err:
+  thd->variables.sql_mode= save_sql_mode;
   free_io_cache(from);
   *copied= found_count;
   *deleted=delete_count;