MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Alexey Kopytov Date:November 26 2007 3:36pm
Subject:bk commit into 5.0 tree (kaa:1.2570) BUG#9481
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of kaa. When kaa 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@stripped, 2007-11-26 18:36:05+03:00, kaa@polly.(none) +3 -0
  5.0 version of the fix for bug #9481: mysql_insert_id() returns 0 after
  insert ... select.
  
  The 5.0 manual page for mysql_insert_id() does not mention anything
  about INSERT ... SELECT, though its current behavior is incosistent
  with what the manual says about the plain INSERT.
  
  Fixed by changing the AUTO_INCREMENT and mysql_insert_id() handling
  logic in INSERT ... SELECT to be consistent with the INSERT behavior,
  the manual, and the changes in 5.1 introduced by WL3146:
  
  
  - mysql_insert_id() now returns the first automatically generated
  AUTO_INCREMENT value that was successfully inserted by INSERT ... SELECT
  
  -  if an INSERT ... SELECT statement is executed, and no automatically
  generated value is successfully inserted, mysql_insert_id() now returns
  the ID of the last inserted row.

  sql/sql_class.h@stripped, 2007-11-26 18:36:00+03:00, kaa@polly.(none) +2 -1
    Replaced last_insert_id with autoinc_value_of_last_inserted_row to be
    consistent with 5.1 code.

  sql/sql_insert.cc@stripped, 2007-11-26 18:36:00+03:00, kaa@polly.(none) +40 -8
    Revised the AUTO_INCREMENT and mysql_insert_id() handling logic in
    INSERT ... SELECT to be consistent with INSERT behavior, the manual, and
    changes in 5.1 introduced by WL3146:
    
    - mysql_insert_id() now returns the first automatically generated
    AUTO_INCREMENT value that was successfully inserted;
    
    -  if an INSERT ... SELECT statement is executed, and no automatically
    generated value is successfully inserted, mysql_insert_id() now returns
    the ID of the last inserted row.

  tests/mysql_client_test.c@stripped, 2007-11-26 18:36:00+03:00, kaa@polly.(none) +182 -0
    Backported the test cases related to INSERT ... SELECT and
    mysql_insert_id() from WL3146 patch to 5.0.

diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
--- a/sql/sql_class.h	2007-11-10 20:29:11 +03:00
+++ b/sql/sql_class.h	2007-11-26 18:36:00 +03:00
@@ -2048,7 +2048,8 @@ class select_insert :public select_resul
   TABLE_LIST *table_list;
   TABLE *table;
   List<Item> *fields;
-  ulonglong last_insert_id;
+  ulonglong autoinc_value_of_last_inserted_row; // not autogenerated
+  ulonglong autoinc_value_of_first_inserted_row; // autogenerated
   COPY_INFO info;
   bool insert_into_view;
   bool is_bulk_insert_mode;
diff -Nrup a/sql/sql_insert.cc b/sql/sql_insert.cc
--- a/sql/sql_insert.cc	2007-09-27 13:17:13 +04:00
+++ b/sql/sql_insert.cc	2007-11-26 18:36:00 +03:00
@@ -2644,7 +2644,8 @@ select_insert::select_insert(TABLE_LIST 
                              enum_duplicates duplic,
                              bool ignore_check_option_errors)
   :table_list(table_list_par), table(table_par), fields(fields_par),
-   last_insert_id(0),
+   autoinc_value_of_last_inserted_row(0),
+   autoinc_value_of_first_inserted_row(0),
    insert_into_view(table_list_par && table_list_par->view != 0),
    is_bulk_insert_mode(FALSE)
 {
@@ -2902,14 +2903,24 @@ bool select_insert::send_data(List<Item>
     if (table->next_number_field)
     {
       /*
+        If no value has been autogenerated so far, we need to remember the
+        value we just saw, we may need to send it to client in the end.
+      */
+      if (!thd->insert_id_used)
+        autoinc_value_of_last_inserted_row= table->next_number_field->val_int();
+      /*
         Clear auto-increment field for the next record, if triggers are used
         we will clear it twice, but this should be cheap.
       */
       table->next_number_field->reset();
-      if (!last_insert_id && thd->insert_id_used)
-        last_insert_id= thd->last_insert_id;
+      if (!autoinc_value_of_last_inserted_row && thd->insert_id_used)
+        autoinc_value_of_last_inserted_row= thd->last_insert_id;
     }
   }
+
+  if (thd->insert_id_used && !autoinc_value_of_first_inserted_row)
+    autoinc_value_of_first_inserted_row= thd->last_insert_id;
+  
   DBUG_RETURN(error);
 }
 
@@ -2938,6 +2949,7 @@ bool select_insert::send_eof()
 {
   int error, error2;
   bool changed, transactional_table= table->file->has_transactions();
+  ulonglong id;
   DBUG_ENTER("select_insert::send_eof");
 
   error= (!thd->prelocked_mode) ? table->file->end_bulk_insert():0;
@@ -2959,8 +2971,17 @@ bool select_insert::send_eof()
   DBUG_ASSERT(transactional_table || !changed || 
               thd->transaction.stmt.modified_non_trans_table);
 
-  if (last_insert_id)
-    thd->insert_id(info.copied ? last_insert_id : 0);		// For binary log
+  // For binary log
+  if (autoinc_value_of_last_inserted_row)
+  {
+    if (info.copied)
+      thd->insert_id(autoinc_value_of_last_inserted_row);
+    else
+    {
+      autoinc_value_of_first_inserted_row= 0;
+      thd->insert_id(0);
+    }
+  }
   /* Write to binlog before commiting transaction */
   if (mysql_bin_log.is_open())
   {
@@ -2987,7 +3008,9 @@ bool select_insert::send_eof()
   thd->row_count_func= info.copied + info.deleted +
                        ((thd->client_capabilities & CLIENT_FOUND_ROWS) ?
                         info.touched : info.updated);
-  ::send_ok(thd, (ulong) thd->row_count_func, last_insert_id, buff);
+  id= autoinc_value_of_first_inserted_row > 0 ?
+    autoinc_value_of_first_inserted_row : thd->last_insert_id;
+  ::send_ok(thd, (ulong) thd->row_count_func, id, buff);
   DBUG_RETURN(0);
 }
 
@@ -3016,8 +3039,17 @@ void select_insert::abort()
   if ((changed= info.copied || info.deleted || info.updated) &&
       !transactional_table)
   {
-    if (last_insert_id)
-      thd->insert_id(last_insert_id);		// For binary log
+    // For binary log
+    if (autoinc_value_of_last_inserted_row)
+    {
+      if (info.copied)
+        thd->insert_id(autoinc_value_of_last_inserted_row);
+      else
+      {
+        autoinc_value_of_first_inserted_row= 0;
+        thd->insert_id(0);
+      }
+    }
     if (mysql_bin_log.is_open())
     {
       Query_log_event qinfo(thd, thd->query, thd->query_length,
diff -Nrup a/tests/mysql_client_test.c b/tests/mysql_client_test.c
--- a/tests/mysql_client_test.c	2007-10-30 15:41:20 +03:00
+++ b/tests/mysql_client_test.c	2007-11-26 18:36:00 +03:00
@@ -15200,6 +15200,187 @@ static void test_bug14169()
 
 
 /*
+   Test that mysql_insert_id() behaves as documented in our manual
+*/
+
+static void test_mysql_insert_id()
+{
+  my_ulonglong res;
+  int rc;
+
+  myheader("test_mysql_insert_id");
+
+  rc= mysql_query(mysql, "drop table if exists t1");
+  myquery(rc);
+  /* table without auto_increment column */
+  rc= mysql_query(mysql, "create table t1 (f1 int, f2 varchar(255), key(f1))");
+  myquery(rc);
+  rc= mysql_query(mysql, "insert into t1 values (1,'a')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "insert into t1 values (null,'b')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "insert into t1 select 5,'c'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "insert into t1 select null,'d'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "insert into t1 values (null,last_insert_id(300))");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 300);
+  rc= mysql_query(mysql, "insert into t1 select null,last_insert_id(400)");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  /*
+    Behaviour change: old code used to return 0; but 400 is consistent
+    with INSERT VALUES, and the manual's section of mysql_insert_id() does not
+    say INSERT SELECT should be different.
+  */
+  DIE_UNLESS(res == 400);
+
+  /* table with auto_increment column */
+  rc= mysql_query(mysql, "create table t2 (f1 int not null primary key auto_increment, f2 varchar(255))");
+  myquery(rc);
+  rc= mysql_query(mysql, "insert into t2 values (1,'a')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 1);
+  /* this should not influence next INSERT if it doesn't have auto_inc */
+  rc= mysql_query(mysql, "insert into t1 values (10,'e')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+
+  rc= mysql_query(mysql, "insert into t2 values (null,'b')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 2);
+  rc= mysql_query(mysql, "insert into t2 select 5,'c'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  /*
+    Manual says that for multirow insert this should have been 5, but does not
+    say for INSERT SELECT. This is a behaviour change: old code used to return
+    0. We try to be consistent with INSERT VALUES.
+  */
+  DIE_UNLESS(res == 5);
+  rc= mysql_query(mysql, "insert into t2 select null,'d'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 6);
+  /* with more than one row */
+  rc= mysql_query(mysql, "insert into t2 values (10,'a'),(11,'b')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 11);
+  rc= mysql_query(mysql, "insert into t2 select 12,'a' union select 13,'b'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  /*
+    Manual says that for multirow insert this should have been 13, but does
+    not say for INSERT SELECT. This is a behaviour change: old code used to
+    return 0. We try to be consistent with INSERT VALUES.
+  */
+  DIE_UNLESS(res == 13);
+  rc= mysql_query(mysql, "insert into t2 values (null,'a'),(null,'b')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 14);
+  rc= mysql_query(mysql, "insert into t2 select null,'a' union select null,'b'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 16);
+  rc= mysql_query(mysql, "insert into t2 select 12,'a' union select 13,'b'");
+  myquery_r(rc);
+  rc= mysql_query(mysql, "insert ignore into t2 select 12,'a' union select 13,'b'");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "insert into t2 values (12,'a'),(13,'b')");
+  myquery_r(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "insert ignore into t2 values (12,'a'),(13,'b')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  /* mixing autogenerated and explicit values */
+  rc= mysql_query(mysql, "insert into t2 values (null,'e'),(12,'a'),(13,'b')");
+  myquery_r(rc);
+  rc= mysql_query(mysql, "insert into t2 values (null,'e'),(12,'a'),(13,'b'),(25,'g')");
+  myquery_r(rc);
+  rc= mysql_query(mysql, "insert into t2 values (null,last_insert_id(300))");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  /*
+    according to the manual, this might be 20 or 300, but it looks like
+    auto_increment column takes priority over last_insert_id().
+  */
+  DIE_UNLESS(res == 20);
+  /* If first autogenerated number fails and 2nd works: */
+  rc= mysql_query(mysql, "drop table t2");
+  myquery(rc);
+  rc= mysql_query(mysql, "create table t2 (f1 int not null primary key "
+                  "auto_increment, f2 varchar(255), unique (f2))");
+  myquery(rc);
+  rc= mysql_query(mysql, "insert into t2 values (null,'e')");
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 1);
+  rc= mysql_query(mysql, "insert ignore into t2 values (null,'e'),(null,'a'),(null,'e')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 2);
+  /* If autogenerated fails and explicit works: */
+  rc= mysql_query(mysql, "insert ignore into t2 values (null,'e'),(12,'c'),(null,'d')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 3);
+  /* UPDATE may update mysql_insert_id() if it uses LAST_INSERT_ID(#) */
+  rc= mysql_query(mysql, "update t2 set f1=14 where f1=12");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "update t2 set f1=NULL where f1=14");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  rc= mysql_query(mysql, "update t2 set f2=last_insert_id(372) where f1=0");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 372);
+  /* check that LAST_INSERT_ID() does not update mysql_insert_id(): */
+  rc= mysql_query(mysql, "insert into t2 values (null,'g')");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 15);
+  rc= mysql_query(mysql, "update t2 set f2=(@li:=last_insert_id()) where f1=15");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 0);
+  /*
+    Behaviour change: now if ON DUPLICATE KEY UPDATE updates a row,
+    mysql_insert_id() returns the id of the row, instead of not being
+    affected.
+  */
+  rc= mysql_query(mysql, "insert into t2 values (null,@li) on duplicate key "
+                  "update f2=concat('we updated ',f2)");
+  myquery(rc);
+  res= mysql_insert_id(mysql);
+  DIE_UNLESS(res == 15);
+
+  rc= mysql_query(mysql, "drop table t1,t2");
+  myquery(rc);
+}
+
+
+/*
   Bug#20152: mysql_stmt_execute() writes to MYSQL_TYPE_DATE buffer
 */
 
@@ -16237,6 +16418,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug17667", test_bug17667 },
   { "test_bug19671", test_bug19671 },
   { "test_bug15752", test_bug15752 },
+  { "test_mysql_insert_id", test_mysql_insert_id },
   { "test_bug21206", test_bug21206 },
   { "test_bug21726", test_bug21726 },
   { "test_bug15518", test_bug15518 },
Thread
bk commit into 5.0 tree (kaa:1.2570) BUG#9481Alexey Kopytov26 Nov