List:Commits« Previous MessageNext Message »
From:kroki Date:October 6 2006 9:34am
Subject:bk commit into 4.1 tree (kroki:1.2554) BUG#21726
View as plain text  
Below is the list of changes that have just been committed into a local
4.1 repository of tomash. When tomash 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, 2006-10-06 13:34:07+04:00, kroki@stripped +13 -0
  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@stripped, 2006-10-06 13:34:04+04:00, kroki@stripped +27 -0
    Add result for bug#21726: Incorrect result with multiple invocations
    of LAST_INSERT_ID.

  mysql-test/t/rpl_insert_id.test@stripped, 2006-10-06 13:34:04+04:00, kroki@stripped +32 -0
    Add test case for bug#21726: Incorrect result with multiple invocations
    of LAST_INSERT_ID.

  sql/item_func.cc@stripped, 2006-10-06 13:34:04+04:00, kroki@stripped +28 -3
    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@stripped, 2006-10-06 13:34:04+04:00, kroki@stripped +1 -0
    Add declaration of Item_func_last_insert_id::fix_fields().

  sql/log_event.cc@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +0 -1
    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@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +6 -2
    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@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +33 -15
    Update comments.
    Remove THD::insert_id(), as it has lost its purpose now.

  sql/sql_insert.cc@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +3 -5
    Now it is OK to read THD::last_insert_id directly.

  sql/sql_load.cc@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +4 -8
    Now it is OK to read THD::last_insert_id directly.

  sql/sql_parse.cc@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +6 -0
    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@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +9 -2
    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@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +2 -2
    Now it is OK to read THD::last_insert_id directly.

  tests/mysql_client_test.c@stripped, 2006-10-06 13:34:05+04:00, kroki@stripped +38 -0
    Add test case for bug#21726: Incorrect result with multiple invocations
    of LAST_INSERT_ID.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	kroki
# Host:	moonlight.intranet
# Root:	/home/tomash/src/mysql_ab/mysql-4.1-bug21726

--- 1.262/sql/item_func.cc	2006-10-06 13:34:13 +04:00
+++ 1.263/sql/item_func.cc	2006-10-06 13:34:13 +04:00
@@ -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,11 +2263,12 @@ longlong Item_func_last_insert_id::val_i
     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->last_insert_id_used ? thd->current_insert_id : thd->insert_id();
+
+  return thd->current_insert_id;
 }
+
 
 /* This function is just used to test speed of different functions */
 

--- 1.131/sql/item_func.h	2006-10-06 13:34:13 +04:00
+++ 1.132/sql/item_func.h	2006-10-06 13:34:13 +04:00
@@ -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

--- 1.188/sql/log_event.cc	2006-10-06 13:34:13 +04:00
+++ 1.189/sql/log_event.cc	2006-10-06 13:34:13 +04:00
@@ -2255,7 +2255,6 @@ int Intvar_log_event::exec_event(struct 
 {
   switch (type) {
   case LAST_INSERT_ID_EVENT:
-    thd->last_insert_id_used = 1;
     thd->last_insert_id = val;
     break;
   case INSERT_ID_EVENT:

--- 1.288/sql/sql_class.h	2006-10-06 13:34:13 +04:00
+++ 1.289/sql/sql_class.h	2006-10-06 13:34:13 +04:00
@@ -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=#
-    or SELECT LAST_INSERT_ID(#).  Used for binary log and returned by
-    LAST_INSERT_ID()
+    At the beginning of the statement last_insert_id holds the first
+    generated value of the previous statement.  During statement
+    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
-    statement.  When this is set, last_insert_id_used is set to true.
+    current_insert_id remembers the first generated value of the
+    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;
@@ -995,15 +1022,6 @@ public:
     last_insert_id= id_arg;
     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)
   {

--- 1.172/sql/sql_insert.cc	2006-10-06 13:34:13 +04:00
+++ 1.173/sql/sql_insert.cc	2006-10-06 13:34:13 +04:00
@@ -374,10 +374,8 @@ int mysql_insert(THD *thd,TABLE_LIST *ta
     if (error)
       break;
     /*
-      If auto_increment values are used, save the first one
-       for 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 auto_increment values are used, save the first one for
+      LAST_INSERT_ID() and for the update log.
     */
     if (! id && thd->insert_id_used)
     {						// Get auto increment value
@@ -1687,7 +1685,7 @@ bool select_insert::send_data(List<Item>
     {
       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);

--- 1.87/sql/sql_load.cc	2006-10-06 13:34:13 +04:00
+++ 1.88/sql/sql_load.cc	2006-10-06 13:34:13 +04:00
@@ -466,10 +466,8 @@ read_fixed_length(THD *thd,COPY_INFO &in
     if (write_record(table,&info))
       DBUG_RETURN(1);
     /*
-      If auto_increment values are used, save the first one
-       for 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 auto_increment values are used, save the first one for
+      LAST_INSERT_ID() and for the binary/update log.
     */
     if (!id && thd->insert_id_used)
       id= thd->last_insert_id;
@@ -572,10 +570,8 @@ read_sep_field(THD *thd,COPY_INFO &info,
     if (write_record(table,&info))
       DBUG_RETURN(1);
     /*
-      If auto_increment values are used, save the first one
-       for 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 auto_increment values are used, save the first one for
+      LAST_INSERT_ID() and for the binary/update log.
     */
     if (!id && thd->insert_id_used)
       id= thd->last_insert_id;

--- 1.488/sql/sql_parse.cc	2006-10-06 13:34:13 +04:00
+++ 1.489/sql/sql_parse.cc	2006-10-06 13:34:13 +04:00
@@ -1978,6 +1978,12 @@ mysql_execute_command(THD *thd)
   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
     that is not a SHOW command or a select that only access local

--- 1.460/sql/sql_select.cc	2006-10-06 13:34:13 +04:00
+++ 1.461/sql/sql_select.cc	2006-10-06 13:34:13 +04:00
@@ -4820,7 +4820,7 @@ remove_eq_conds(THD *thd, COND *cond, It
       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, It
 	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);
 	}

--- 1.156/sql/sql_update.cc	2006-10-06 13:34:13 +04:00
+++ 1.157/sql/sql_update.cc	2006-10-06 13:34:13 +04:00
@@ -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;
 }

--- 1.13/mysql-test/r/rpl_insert_id.result	2006-10-06 13:34:13 +04:00
+++ 1.14/mysql-test/r/rpl_insert_id.result	2006-10-06 13:34:13 +04:00
@@ -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
 #

--- 1.13/mysql-test/t/rpl_insert_id.test	2006-10-06 13:34:13 +04:00
+++ 1.14/mysql-test/t/rpl_insert_id.test	2006-10-06 13:34:13 +04:00
@@ -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 #

--- 1.186/sql/set_var.cc	2006-10-06 13:34:13 +04:00
+++ 1.187/sql/set_var.cc	2006-10-06 13:34:13 +04:00
@@ -2404,8 +2404,12 @@ bool sys_var_last_insert_id::update(THD 
 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;
 }
 
 

--- 1.170/tests/mysql_client_test.c	2006-10-06 13:34:13 +04:00
+++ 1.171/tests/mysql_client_test.c	2006-10-06 13:34:13 +04:00
@@ -11909,6 +11909,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 }
 };
 
Thread
bk commit into 4.1 tree (kroki:1.2554) BUG#21726kroki6 Oct