From f603c1cce8fb44772f30d69ab402b036d1fb1593 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 6 Oct 2006 13:34:07 +0400 Subject: [PATCH] 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 "= ", 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 | 27 +++++++++++++++++ mysql-test/t/rpl_insert_id.test | 32 +++++++++++++++++++++ sql/item_func.cc | 31 ++++++++++++++++++-- sql/item_func.h | 1 + sql/log_event.cc | 1 - sql/set_var.cc | 8 ++++-- sql/sql_class.h | 48 +++++++++++++++++++++---------- sql/sql_insert.cc | 8 ++---- sql/sql_load.cc | 12 +++----- sql/sql_parse.cc | 6 ++++ sql/sql_select.cc | 11 +++++-- sql/sql_update.cc | 4 +-- tests/mysql_client_test.c | 38 ++++++++++++++++++++++++ 13 files changed, 189 insertions(+), 38 deletions(-) diff --git a/mysql-test/r/rpl_insert_id.result b/mysql-test/r/rpl_insert_id.result index fbdc9dc06cf..43aba68d041 100644 --- a/mysql-test/r/rpl_insert_id.result +++ b/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 # diff --git a/mysql-test/t/rpl_insert_id.test b/mysql-test/t/rpl_insert_id.test index 327094a1394..7fb514fb7af 100644 --- a/mysql-test/t/rpl_insert_id.test +++ b/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 # diff --git a/sql/item_func.cc b/sql/item_func.cc index 91ccef6511f..6f8eed42e51 100644 --- a/sql/item_func.cc +++ b/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->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 */ longlong Item_func_benchmark::val_int() diff --git a/sql/item_func.h b/sql/item_func.h index bc6c955b99f..467b88eda76 100644 --- a/sql/item_func.h +++ b/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 diff --git a/sql/log_event.cc b/sql/log_event.cc index 19c32b2d28e..412ebbce0ac 100644 --- a/sql/log_event.cc +++ b/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: diff --git a/sql/set_var.cc b/sql/set_var.cc index 4acedc7bcbd..88e120632e2 100644 --- a/sql/set_var.cc +++ b/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; } diff --git a/sql/sql_class.h b/sql/sql_class.h index a995a492bc8..5c4c3af7acb 100644 --- a/sql/sql_class.h +++ b/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=# - 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; @@ -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; diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 283fe571d53..43c7e5d456f 100644 --- a/sql/sql_insert.cc +++ b/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 - 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 &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); diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 4e6c458cc43..48862506729 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -466,10 +466,8 @@ read_fixed_length(THD *thd,COPY_INFO &info,TABLE *table,List &fields, 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,TABLE *table, 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; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 98199ed22f1..cb2fa0f7014 100644 --- a/sql/sql_parse.cc +++ b/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 diff --git a/sql/sql_select.cc b/sql/sql_select.cc index da96c98cd4f..117795059f0 100644 --- a/sql/sql_select.cc +++ b/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); } diff --git a/sql/sql_update.cc b/sql/sql_update.cc index af4ba8025f9..51643fc611d 100644 --- a/sql/sql_update.cc +++ b/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; } diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 9fabde993b8..6ae9dcb9476 100644 --- a/tests/mysql_client_test.c +++ b/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 } };