BUG#21726: Incorrect result with multiple invocations of LAST_INSERT_ID.

Note: bug#21726 does not directly apply to 4.1, as it doesn't have stored
procedures.  However, 4.1 had some bugs that were fixed in 5.0 by the
patch for bug#21726, and this patch is a backport of those fixes.
Namely, in 4.1 it fixes:

  - LAST_INSERT_ID(expr) didn't return value of expr (4.1 specific).

  - LAST_INSERT_ID() could return the value generated by current
    statement if the call happens after the generation, like in

      CREATE TABLE t1 (i INT AUTO_INCREMENT PRIMARY KEY, j INT);
      INSERT INTO t1 VALUES (NULL, 0), (NULL, LAST_INSERT_ID());

  - Redundant binary log LAST_INSERT_ID_EVENTs could be generated.


mysql-test/r/rpl_insert_id.result:
  Add result for bug#21726: Incorrect result with multiple invocations
  of LAST_INSERT_ID.
mysql-test/t/rpl_insert_id.test:
  Add test case for bug#21726: Incorrect result with multiple invocations
  of LAST_INSERT_ID.
sql/item_func.cc:
  Add implementation of Item_func_last_insert_id::fix_fields(), where we
  set THD::last_insert_id_used when statement calls LAST_INSERT_ID().
  In Item_func_last_insert_id::val_int(), return THD::current_insert_id
  if called like LAST_INSERT_ID(), otherwise return value of argument if
  called like LAST_INSERT_ID(expr).
sql/item_func.h:
  Add declaration of Item_func_last_insert_id::fix_fields().
sql/log_event.cc:
  Do not set THD::last_insert_id_used on LAST_INSERT_ID_EVENT.  Though we
  know the statement will call LAST_INSERT_ID(), it wasn't called yet.
sql/set_var.cc:
  In sys_var_last_insert_id::value_ptr(), set THD::last_insert_id_used,
  and return THD::current_insert_id for @@LAST_INSERT_ID.
sql/sql_class.h:
  Update comments.
  Remove THD::insert_id(), as it has lost its purpose now.
sql/sql_insert.cc:
  Now it is OK to read THD::last_insert_id directly.
sql/sql_load.cc:
  Now it is OK to read THD::last_insert_id directly.
sql/sql_parse.cc:
  In mysql_execute_command(), remember THD::last_insert_id (first
  generated value of the previous statement) in THD::current_insert_id,
  which then will be returned for LAST_INSERT_ID() and @@LAST_INSERT_ID.
sql/sql_select.cc:
  If "IS NULL" is replaced with "= <LAST_INSERT_ID>", use right value,
  which is THD::current_insert_id, and also set THD::last_insert_id_used
  to issue binary log LAST_INSERT_ID_EVENT.
sql/sql_update.cc:
  Now it is OK to read THD::last_insert_id directly.
tests/mysql_client_test.c:
  Add test case for bug#21726: Incorrect result with multiple invocations
  of LAST_INSERT_ID.

mysql-test/r/rpl_insert_id.result

@@ -108,6 +108,33 @@ a
 1
 drop table t1;
 drop table t2;
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (
+i INT NOT NULL AUTO_INCREMENT PRIMARY KEY,
+j INT DEFAULT 0
+);
+INSERT INTO t1 VALUES (NULL, -1);
+INSERT INTO t1 VALUES (NULL, LAST_INSERT_ID()), (NULL, LAST_INSERT_ID(5)),
+(NULL, @@LAST_INSERT_ID);
+INSERT INTO t1 VALUES (NULL, 0), (NULL, LAST_INSERT_ID());
+UPDATE t1 SET j= -1 WHERE i IS NULL;
+SELECT * FROM t1;
+i	j
+1	-1
+2	1
+3	5
+4	1
+5	-1
+6	2
+SELECT * FROM t1;
+i	j
+1	-1
+2	1
+3	5
+4	1
+5	-1
+6	2
+DROP TABLE t1;
 #
 # End of 4.1 tests
 #

mysql-test/t/rpl_insert_id.test

@@ -108,6 +108,38 @@ drop table t1;
 drop table t2;
 sync_slave_with_master;
 
+
+#
+# BUG#21726: Incorrect result with multiple invocations of
+# LAST_INSERT_ID
+#
+connection master;
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (
+    i INT NOT NULL AUTO_INCREMENT PRIMARY KEY,
+    j INT DEFAULT 0
+);
+
+INSERT INTO t1 VALUES (NULL, -1);
+INSERT INTO t1 VALUES (NULL, LAST_INSERT_ID()), (NULL, LAST_INSERT_ID(5)),
+                      (NULL, @@LAST_INSERT_ID);
+# Test replication of substitution "IS NULL" -> "= LAST_INSERT_ID".
+INSERT INTO t1 VALUES (NULL, 0), (NULL, LAST_INSERT_ID());
+UPDATE t1 SET j= -1 WHERE i IS NULL;
+
+SELECT * FROM t1;
+
+sync_slave_with_master;
+SELECT * FROM t1;
+
+connection master;
+DROP TABLE t1;
+
+
 --echo #
 --echo # End of 4.1 tests
 --echo #

sql/item_func.cc

@@ -2230,6 +2230,30 @@ longlong Item_func_release_lock::val_int()
 }
 
 
+bool Item_func_last_insert_id::fix_fields(THD *thd, TABLE_LIST *tables,
+                                          Item **ref)
+{
+  DBUG_ASSERT(fixed == 0);
+
+  if (Item_int_func::fix_fields(thd, tables, ref))
+    return TRUE;
+
+  if (arg_count == 0)
+  {
+    /*
+      As this statement calls LAST_INSERT_ID(), set
+      THD::last_insert_id_used.
+    */
+    thd->last_insert_id_used= TRUE;
+    null_value= FALSE;
+  }
+
+  thd->lex->uncacheable(UNCACHEABLE_SIDEEFFECT);
+
+  return FALSE;
+}
+
+
 longlong Item_func_last_insert_id::val_int()
 {
   DBUG_ASSERT(fixed == 1);
@@ -2239,12 +2263,13 @@ longlong Item_func_last_insert_id::val_int()
     longlong value=args[0]->val_int();
     thd->insert_id(value);
     null_value=args[0]->null_value;
+    return value;
   }
-  else
+
-    thd->lex->uncacheable(UNCACHEABLE_SIDEEFFECT);
+  return thd->current_insert_id;
-  return thd->last_insert_id_used ? thd->current_insert_id : thd->insert_id();
 }
 
+
 /* This function is just used to test speed of different functions */
 
 longlong Item_func_benchmark::val_int()

sql/item_func.h

@@ -758,6 +758,7 @@ public:
   longlong val_int();
   const char *func_name() const { return "last_insert_id"; }
   void fix_length_and_dec() { if (arg_count) max_length= args[0]->max_length; }
+  bool fix_fields(THD *thd, TABLE_LIST *tables, Item **ref);
 };
 
 class Item_func_benchmark :public Item_int_func

sql/log_event.cc

@@ -2255,7 +2255,6 @@ int Intvar_log_event::exec_event(struct st_relay_log_info* rli)
 {
   switch (type) {
   case LAST_INSERT_ID_EVENT:
-    thd->last_insert_id_used = 1;
     thd->last_insert_id = val;
     break;
   case INSERT_ID_EVENT:

sql/set_var.cc

@@ -2404,8 +2404,12 @@ bool sys_var_last_insert_id::update(THD *thd, set_var *var)
 byte *sys_var_last_insert_id::value_ptr(THD *thd, enum_var_type type,
 					LEX_STRING *base)
 {
-  thd->sys_var_tmp.long_value= (long) thd->insert_id();
+  /*
-  return (byte*) &thd->last_insert_id;
+    As this statement reads @@LAST_INSERT_ID, set
+    THD::last_insert_id_used.
+  */
+  thd->last_insert_id_used= TRUE;
+  return (byte*) &thd->current_insert_id;
 }
 
 

sql/sql_class.h

@@ -835,17 +835,29 @@ public:
     generated auto_increment value in handler.cc
   */
   ulonglong  next_insert_id;
+
   /*
-    The insert_id used for the last statement or set by SET LAST_INSERT_ID=#
+    At the beginning of the statement last_insert_id holds the first
-    or SELECT LAST_INSERT_ID(#).  Used for binary log and returned by
+    generated value of the previous statement.  During statement
-    LAST_INSERT_ID()
+    execution it is updated to the value just generated, but then
+    restored to the value that was generated first, so for the next
+    statement it will again be "the first generated value of the
+    previous statement".
+
+    It may also be set with "LAST_INSERT_ID(expr)" or
+    "@@LAST_INSERT_ID= expr", but the effect of such setting will be
+    seen only in the next statement.
   */
   ulonglong  last_insert_id;
+
   /*
-    Set to the first value that LAST_INSERT_ID() returned for the last
+    current_insert_id remembers the first generated value of the
-    statement.  When this is set, last_insert_id_used is set to true.
+    previous statement, and does not change during statement
+    execution.  Its value returned from LAST_INSERT_ID() and
+    @@LAST_INSERT_ID.
   */
   ulonglong  current_insert_id;
+
   ulonglong  limit_found_rows;
   ha_rows    cuted_fields,
              sent_row_count, examined_row_count;
@@ -896,7 +908,22 @@ public:
   bool	     locked, some_tables_deleted;
   bool       last_cuted_field;
   bool	     no_errors, password, is_fatal_error;
-  bool	     query_start_used,last_insert_id_used,insert_id_used,rand_used;
+  bool	     query_start_used, rand_used;
+
+  /*
+    last_insert_id_used is set when current statement calls
+    LAST_INSERT_ID() or reads @@LAST_INSERT_ID, so that binary log
+    LAST_INSERT_ID_EVENT be generated.
+  */
+  bool	     last_insert_id_used;
+
+  /*
+    insert_id_used is set when current statement updates
+    THD::last_insert_id, so that binary log INSERT_ID_EVENT be
+    generated.
+  */
+  bool       insert_id_used;
+
   /* for IS NULL => = last_insert_id() fix in remove_eq_conds() */
   bool       substitute_null_with_insert_id;
   bool	     time_zone_used;
@@ -996,15 +1023,6 @@ public:
     insert_id_used=1;
     substitute_null_with_insert_id= TRUE;
   }
-  inline ulonglong insert_id(void)
-  {
-    if (!last_insert_id_used)
-    {      
-      last_insert_id_used=1;
-      current_insert_id=last_insert_id;
-    }
-    return last_insert_id;
-  }
   inline ulonglong found_rows(void)
   {
     return limit_found_rows;

sql/sql_insert.cc

@@ -374,10 +374,8 @@ int mysql_insert(THD *thd,TABLE_LIST *table_list,
     if (error)
       break;
     /*
-      If auto_increment values are used, save the first one
+      If auto_increment values are used, save the first one for
-       for LAST_INSERT_ID() and for the update log.
+      LAST_INSERT_ID() and for the update log.
-       We can't use insert_id() as we don't want to touch the
-       last_insert_id_used flag.
     */
     if (! id && thd->insert_id_used)
     {						// Get auto increment value
@@ -1687,7 +1685,7 @@ bool select_insert::send_data(List<Item> &values)
     {
       table->next_number_field->reset();
       if (! last_insert_id && thd->insert_id_used)
-        last_insert_id=thd->insert_id();
+        last_insert_id= thd->last_insert_id;
     }
   }
   DBUG_RETURN(error);

sql/sql_load.cc

@@ -466,10 +466,8 @@ read_fixed_length(THD *thd,COPY_INFO &info,TABLE *table,List<Item> &fields,
     if (write_record(table,&info))
       DBUG_RETURN(1);
     /*
-      If auto_increment values are used, save the first one
+      If auto_increment values are used, save the first one for
-       for LAST_INSERT_ID() and for the binary/update log.
+      LAST_INSERT_ID() and for the binary/update log.
-       We can't use insert_id() as we don't want to touch the
-       last_insert_id_used flag.
     */
     if (!id && thd->insert_id_used)
       id= thd->last_insert_id;
@@ -572,10 +570,8 @@ read_sep_field(THD *thd,COPY_INFO &info,TABLE *table,
     if (write_record(table,&info))
       DBUG_RETURN(1);
     /*
-      If auto_increment values are used, save the first one
+      If auto_increment values are used, save the first one for
-       for LAST_INSERT_ID() and for the binary/update log.
+      LAST_INSERT_ID() and for the binary/update log.
-       We can't use insert_id() as we don't want to touch the
-       last_insert_id_used flag.
     */
     if (!id && thd->insert_id_used)
       id= thd->last_insert_id;

sql/sql_parse.cc

@@ -1977,6 +1977,12 @@ mysql_execute_command(THD *thd)
   SELECT_LEX_UNIT *unit= &lex->unit;
   DBUG_ENTER("mysql_execute_command");
 
+  /*
+    Remember first generated insert id value of the previous
+    statement.
+  */
+  thd->current_insert_id= thd->last_insert_id;
+
   /*
     Reset warning count for each query that uses tables
     A better approach would be to reset this for any commands

sql/sql_select.cc

@@ -4820,7 +4820,7 @@ remove_eq_conds(THD *thd, COND *cond, Item::cond_result *cond_value)
       Field *field=((Item_field*) args[0])->field;
       if (field->flags & AUTO_INCREMENT_FLAG && !field->table->maybe_null &&
 	  (thd->options & OPTION_AUTO_IS_NULL) &&
-	  thd->insert_id() && thd->substitute_null_with_insert_id)
+	  thd->current_insert_id && thd->substitute_null_with_insert_id)
       {
 #ifdef HAVE_QUERY_CACHE
 	query_cache_abort(&thd->net);
@@ -4828,9 +4828,16 @@ remove_eq_conds(THD *thd, COND *cond, Item::cond_result *cond_value)
 	COND *new_cond;
 	if ((new_cond= new Item_func_eq(args[0],
 					new Item_int("last_insert_id()",
-						     thd->insert_id(),
+						     thd->current_insert_id,
 						     21))))
 	{
+          /*
+            Set THD::last_insert_id_used manually, as this statement
+            uses LAST_INSERT_ID() in a sense, and should issue
+            LAST_INSERT_ID_EVENT.
+          */
+          thd->last_insert_id_used= TRUE;
+
 	  cond=new_cond;
 	  cond->fix_fields(thd, 0, &cond);
 	}

sql/sql_update.cc

@@ -408,7 +408,7 @@ int mysql_update(THD *thd,
 	    (ulong) thd->cuted_fields);
     send_ok(thd,
 	    (thd->client_capabilities & CLIENT_FOUND_ROWS) ? found : updated,
-	    thd->insert_id_used ? thd->insert_id() : 0L,buff);
+	    thd->insert_id_used ? thd->last_insert_id : 0L,buff);
     DBUG_PRINT("info",("%d records updated",updated));
   }
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;		/* calc cuted fields */
@@ -1318,6 +1318,6 @@ bool multi_update::send_eof()
 	  (ulong) thd->cuted_fields);
   ::send_ok(thd,
 	    (thd->client_capabilities & CLIENT_FOUND_ROWS) ? found : updated,
-	    thd->insert_id_used ? thd->insert_id() : 0L,buff);
+	    thd->insert_id_used ? thd->last_insert_id : 0L,buff);
   return 0;
 }

tests/mysql_client_test.c

@@ -11908,6 +11908,43 @@ static void test_bug20152()
 }
 
 
+/*
+  Bug#21726: Incorrect result with multiple invocations of
+  LAST_INSERT_ID
+
+  Test that client gets updated value of insert_id on UPDATE that uses
+  LAST_INSERT_ID(expr).
+*/
+static void test_bug21726()
+{
+  const char *update_query = "UPDATE t1 SET i= LAST_INSERT_ID(i + 1)";
+  int rc;
+  my_ulonglong insert_id;
+
+  DBUG_ENTER("test_bug21726");
+  myheader("test_bug21726");
+
+  rc= mysql_query(mysql, "DROP TABLE IF EXISTS t1");
+  myquery(rc);
+  rc= mysql_query(mysql, "CREATE TABLE t1 (i INT)");
+  myquery(rc);
+  rc= mysql_query(mysql, "INSERT INTO t1 VALUES (1)");
+  myquery(rc);
+
+  rc= mysql_query(mysql, update_query);
+  myquery(rc);
+  insert_id= mysql_insert_id(mysql);
+  DIE_UNLESS(insert_id == 2);
+
+  rc= mysql_query(mysql, update_query);
+  myquery(rc);
+  insert_id= mysql_insert_id(mysql);
+  DIE_UNLESS(insert_id == 3);
+
+  DBUG_VOID_RETURN;
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -12134,6 +12171,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug12925", test_bug12925 },
   { "test_bug15613", test_bug15613 },
   { "test_bug20152", test_bug20152 },
+  { "test_bug21726", test_bug21726 },
   { 0, 0 }
 };