Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the

NO_AUTO_VALUE_ON_ZERO mode.

In the NO_AUTO_VALUE_ON_ZERO mode the table->auto_increment_field_not_null
variable is used to indicate that a non-NULL value was specified by the user
for an auto_increment column. When an INSERT .. ON DUPLICATE updates the
auto_increment field this variable is set to true and stays unchanged for the
next insert operation. This makes the next inserted row sometimes wrongly have
0 as the value of the auto_increment field.

Now the fill_record() function resets the table->auto_increment_field_not_null
variable before filling the record.
The table->auto_increment_field_not_null variable is also reset by the
open_table() function for a case if we missed some auto_increment_field_not_null
handling bug.
Now the table->auto_increment_field_not_null is reset at the end of the
mysql_load() function.

Reset the table->auto_increment_field_not_null variable after each
write_row() call in the copy_data_between_tables() function.




sql/field_conv.cc:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the 
  NO_AUTO_VALUE_ON_ZERO mode.
  A comment is corrected.
sql/handler.cc:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the 
  NO_AUTO_VALUE_ON_ZERO mode.
  Now the handler::update_auto_increment() function doesn't reset the
  table->auto_increment_field_not_null variable as it is done in the
  fill_record() function.
sql/sql_base.cc:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the 
  NO_AUTO_VALUE_ON_ZERO mode.
  Now the fill_record() function resets the table->auto_increment_field_not_null
  variable before filling the record.
  The table->auto_increment_field_not_null variable is also reset by the
  open_table() function for a case if we missed some auto_increment_field_not_null
  handling bug.
sql/sql_insert.cc:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the
  NO_AUTO_VALUE_ON_ZERO mode.
  Now the the table->auto_increment_field_not_null is reset at the end of the
  mysql_insert() an in the select_insert class destructor.
sql/sql_load.cc:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the 
  NO_AUTO_VALUE_ON_ZERO mode.
  Now the table->auto_increment_field_not_null is reset at the end of the
  mysql_load() function.
sql/sql_table.cc:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the
  NO_AUTO_VALUE_ON_ZERO mode.
  Reset the table->auto_increment_field_not_null variable after each
  write_row() call in the copy_data_between_tables() function.
sql/table.h:
  Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the
  NO_AUTO_VALUE_ON_ZERO mode.
  A comment added.
mysql-test/r/insert_update.result:
  Added the test case for the bug#23233: 0 as LAST_INSERT_ID() after
  INSERT .. ON DUPLICATE in the NO_AUTO_VALUE_ON_ZERO mode.
mysql-test/t/insert_update.test:
  Added the test case for the bug#23233: 0 as LAST_INSERT_ID() after
  INSERT .. ON DUPLICATE in the NO_AUTO_VALUE_ON_ZERO mode.

mysql-test/r/insert_update.result

@@ -258,3 +258,81 @@ SELECT LAST_INSERT_ID();
 LAST_INSERT_ID()
 1
 DROP TABLE t1;
+SET SQL_MODE='NO_AUTO_VALUE_ON_ZERO';
+CREATE TABLE `t1` (
+`id` int(11) PRIMARY KEY auto_increment,
+`f1` varchar(10) NOT NULL UNIQUE
+);
+INSERT IGNORE INTO t1 (f1) VALUES ("test1")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+INSERT IGNORE INTO t1 (f1) VALUES ("test1")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+LAST_INSERT_ID()
+1
+SELECT * FROM t1;
+id	f1
+1	test1
+INSERT IGNORE INTO t1 (f1) VALUES ("test2")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT * FROM t1;
+id	f1
+1	test1
+2	test2
+INSERT IGNORE INTO t1 (f1) VALUES ("test2")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+LAST_INSERT_ID()
+2
+SELECT * FROM t1;
+id	f1
+1	test1
+2	test2
+INSERT IGNORE INTO t1 (f1) VALUES ("test3")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+LAST_INSERT_ID()
+3
+SELECT * FROM t1;
+id	f1
+1	test1
+2	test2
+3	test3
+DROP TABLE t1;
+CREATE TABLE `t1` (
+`id` int(11) PRIMARY KEY auto_increment,
+`f1` varchar(10) NOT NULL UNIQUE
+);
+INSERT IGNORE INTO t1 (f1) VALUES ("test1")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+LAST_INSERT_ID()
+1
+SELECT * FROM t1;
+id	f1
+1	test1
+INSERT IGNORE INTO t1 (f1) VALUES ("test1"),("test4")
+ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+LAST_INSERT_ID()
+1
+SELECT * FROM t1;
+id	f1
+1	test1
+2	test4
+DROP TABLE t1;
+CREATE TABLE `t1` (
+`id` int(11) PRIMARY KEY auto_increment,
+`f1` varchar(10) NOT NULL UNIQUE,
+tim1 timestamp default '2003-01-01 00:00:00' on update current_timestamp
+);
+INSERT INTO t1 (f1) VALUES ("test1");
+SELECT id, f1 FROM t1;
+id	f1
+1	test1
+REPLACE INTO t1 VALUES (0,"test1",null);
+SELECT id, f1 FROM t1;
+id	f1
+0	test1
+DROP TABLE t1;
+SET SQL_MODE='';

mysql-test/t/insert_update.test

@@ -195,3 +195,55 @@ SELECT LAST_INSERT_ID();
 INSERT t1 (f2) VALUES ('test') ON DUPLICATE KEY UPDATE f1 = LAST_INSERT_ID(f1);
 SELECT LAST_INSERT_ID();
 DROP TABLE t1;
+
+#
+# Bug#23233: 0 as LAST_INSERT_ID() after INSERT .. ON DUPLICATE in the
+#            NO_AUTO_VALUE_ON_ZERO mode.
+#
+SET SQL_MODE='NO_AUTO_VALUE_ON_ZERO';
+CREATE TABLE `t1` (
+  `id` int(11) PRIMARY KEY auto_increment,
+  `f1` varchar(10) NOT NULL UNIQUE
+);
+INSERT IGNORE INTO t1 (f1) VALUES ("test1")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+INSERT IGNORE INTO t1 (f1) VALUES ("test1")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+SELECT * FROM t1;
+INSERT IGNORE INTO t1 (f1) VALUES ("test2")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT * FROM t1;
+INSERT IGNORE INTO t1 (f1) VALUES ("test2")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+SELECT * FROM t1;
+INSERT IGNORE INTO t1 (f1) VALUES ("test3")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+SELECT * FROM t1;
+DROP TABLE t1;
+CREATE TABLE `t1` (
+  `id` int(11) PRIMARY KEY auto_increment,
+  `f1` varchar(10) NOT NULL UNIQUE
+);
+INSERT IGNORE INTO t1 (f1) VALUES ("test1")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+SELECT * FROM t1;
+INSERT IGNORE INTO t1 (f1) VALUES ("test1"),("test4")
+	ON DUPLICATE KEY UPDATE id=LAST_INSERT_ID(id);
+SELECT LAST_INSERT_ID();
+SELECT * FROM t1;
+DROP TABLE t1;
+CREATE TABLE `t1` (
+  `id` int(11) PRIMARY KEY auto_increment,
+  `f1` varchar(10) NOT NULL UNIQUE,
+  tim1 timestamp default '2003-01-01 00:00:00' on update current_timestamp
+);
+INSERT INTO t1 (f1) VALUES ("test1");
+SELECT id, f1 FROM t1;
+REPLACE INTO t1 VALUES (0,"test1",null);
+SELECT id, f1 FROM t1;
+DROP TABLE t1;
+SET SQL_MODE='';

sql/field_conv.cc

@@ -173,7 +173,7 @@ set_field_to_null_with_conversions(Field *field, bool no_conversions)
   if (field == field->table->next_number_field)
   {
     field->table->auto_increment_field_not_null= FALSE;
-    return 0;					// field is set in handler.cc
+    return 0;				  // field is set in fill_record()
   }
   if (current_thd->count_cuted_fields == CHECK_FIELD_WARN)
   {

sql/handler.cc

@@ -1598,7 +1598,6 @@ int handler::update_auto_increment()
   ulonglong nr;
   THD *thd= table->in_use;
   struct system_variables *variables= &thd->variables;
-  bool auto_increment_field_not_null;
   DBUG_ENTER("handler::update_auto_increment");
 
   /*
@@ -1606,14 +1605,11 @@ int handler::update_auto_increment()
     row was not inserted
   */
   thd->prev_insert_id= thd->next_insert_id;
-  auto_increment_field_not_null= table->auto_increment_field_not_null;
-  table->auto_increment_field_not_null= FALSE;
 
   if ((nr= table->next_number_field->val_int()) != 0 ||
-      auto_increment_field_not_null &&
+      table->auto_increment_field_not_null &&
       thd->variables.sql_mode & MODE_NO_AUTO_VALUE_ON_ZERO)
   {
-    /* Clear flag for next row */
     /* Mark that we didn't generate a new value **/
     auto_increment_column_changed=0;
     adjust_next_insert_id_after_explicit_value(nr);

sql/sql_base.cc

@@ -1640,6 +1640,9 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
   table->used_keys= table->s->keys_for_keyread;
   table->fulltext_searched= 0;
   table->file->ft_handler= 0;
+  /* Catch wrong handling of the auto_increment_field_not_null. */
+  DBUG_ASSERT(!table->auto_increment_field_not_null);
+  table->auto_increment_field_not_null= FALSE;
   if (table->timestamp_field)
     table->timestamp_field_type= table->timestamp_field->get_auto_set_type();
   table->pos_in_table_list= table_list;
@@ -5272,6 +5275,11 @@ err_no_arena:
     values        values to fill with
     ignore_errors TRUE if we should ignore errors
 
+  NOTE
+    fill_record() may set table->auto_increment_field_not_null and a
+    caller should make sure that it is reset after their last call to this
+    function.
+
   RETURN
     FALSE   OK
     TRUE    error occured
@@ -5284,27 +5292,52 @@ fill_record(THD * thd, List<Item> &fields, List<Item> &values,
   List_iterator_fast<Item> f(fields),v(values);
   Item *value, *fld;
   Item_field *field;
+  TABLE *table= 0;
   DBUG_ENTER("fill_record");
 
+  /*
+    Reset the table->auto_increment_field_not_null as it is valid for
+    only one row.
+  */
+  if (fields.elements)
+  {
+    /*
+      On INSERT or UPDATE fields are checked to be from the same table,
+      thus we safely can take table from the first field.
+    */
+    fld= (Item_field*)f++;
+    if (!(field= fld->filed_for_view_update()))
+    {
+      my_error(ER_NONUPDATEABLE_COLUMN, MYF(0), fld->name);
+      goto err;
+    }
+    table= field->field->table;
+    table->auto_increment_field_not_null= FALSE;
+    f.rewind();
+  }
   while ((fld= f++))
   {
     if (!(field= fld->filed_for_view_update()))
     {
       my_error(ER_NONUPDATEABLE_COLUMN, MYF(0), fld->name);
-      DBUG_RETURN(TRUE);
+      goto err;
     }
     value=v++;
     Field *rfield= field->field;
-    TABLE *table= rfield->table;
+    table= rfield->table;
     if (rfield == table->next_number_field)
       table->auto_increment_field_not_null= TRUE;
     if ((value->save_in_field(rfield, 0) < 0) && !ignore_errors)
     {
       my_message(ER_UNKNOWN_ERROR, ER(ER_UNKNOWN_ERROR), MYF(0));
-      DBUG_RETURN(TRUE);
+      goto err;
     }
   }
   DBUG_RETURN(thd->net.report_error);
+err:
+  if (table)
+    table->auto_increment_field_not_null= FALSE;
+  DBUG_RETURN(TRUE);
 }
 
 
@@ -5353,6 +5386,11 @@ fill_record_n_invoke_before_triggers(THD *thd, List<Item> &fields,
     values        list of fields
     ignore_errors TRUE if we should ignore errors
 
+  NOTE
+    fill_record() may set table->auto_increment_field_not_null and a
+    caller should make sure that it is reset after their last call to this
+    function.
+
   RETURN
     FALSE   OK
     TRUE    error occured
@@ -5363,19 +5401,38 @@ fill_record(THD *thd, Field **ptr, List<Item> &values, bool ignore_errors)
 {
   List_iterator_fast<Item> v(values);
   Item *value;
+  TABLE *table= 0;
   DBUG_ENTER("fill_record");
 
   Field *field;
+  /*
+    Reset the table->auto_increment_field_not_null as it is valid for
+    only one row.
+  */
+  if (*ptr)
+  {
+    /*
+      On INSERT or UPDATE fields are checked to be from the same table,
+      thus we safely can take table from the first field.
+    */
+    table= (*ptr)->table;
+    table->auto_increment_field_not_null= FALSE;
+  }
   while ((field = *ptr++))
   {
     value=v++;
-    TABLE *table= field->table;
+    table= field->table;
     if (field == table->next_number_field)
       table->auto_increment_field_not_null= TRUE;
     if (value->save_in_field(field, 0) == -1)
-      DBUG_RETURN(TRUE);
+      goto err;
   }
   DBUG_RETURN(thd->net.report_error);
+
+err:
+  if (table)
+    table->auto_increment_field_not_null= FALSE;
+  DBUG_RETURN(TRUE);
 }
 
 

sql/sql_insert.cc

@@ -757,6 +757,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
   table->next_number_field=0;
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;
   thd->next_insert_id=0;			// Reset this if wrongly used
+  table->auto_increment_field_not_null= FALSE;
   if (duplic != DUP_ERROR || ignore)
     table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
   if (duplic == DUP_REPLACE &&
@@ -2571,6 +2572,7 @@ select_insert::~select_insert()
   if (table)
   {
     table->next_number_field=0;
+    table->auto_increment_field_not_null= FALSE;
     table->file->reset();
   }
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;

sql/sql_load.cc

@@ -493,6 +493,7 @@ err:
     mysql_unlock_tables(thd, thd->lock);
     thd->lock=0;
   }
+  table->auto_increment_field_not_null= FALSE;
   thd->abort_on_warning= 0;
   DBUG_RETURN(error);
 }
@@ -589,8 +590,6 @@ read_fixed_length(THD *thd, COPY_INFO &info, TABLE_LIST *table_list,
       {
 	uint length;
 	byte save_chr;
-        if (field == table->next_number_field)
-          table->auto_increment_field_not_null= TRUE;
 	if ((length=(uint) (read_info.row_end-pos)) >
 	    field->field_length)
 	  length=field->field_length;

sql/sql_table.cc

@@ -4058,7 +4058,9 @@ copy_data_between_tables(TABLE *from,TABLE *to,
     {
       copy_ptr->do_copy(copy_ptr);
     }
-    if ((error=to->file->write_row((byte*) to->record[0])))
+    error=to->file->write_row((byte*) to->record[0]);
+    to->auto_increment_field_not_null= FALSE;
+    if (error)
     {
       if (!ignore ||
 	  (error != HA_ERR_FOUND_DUPP_KEY &&

sql/table.h

@@ -273,6 +273,11 @@ struct st_table {
   my_bool no_cache;
   /* To signal that we should reset query_id for tables and cols */
   my_bool clear_query_id;
+  /*
+    To indicate that a non-null value of the auto_increment field
+    was provided by the user or retrieved from the current record.
+    Used only in the MODE_NO_AUTO_VALUE_ON_ZERO mode.
+  */
   my_bool auto_increment_field_not_null;
   my_bool insert_or_update;             /* Can be used by the handler */
   my_bool alias_name_used;		/* true if table_name is alias */