Hi Ramil!
Patch looks good. As far as I'm concerned, it's OK to push.
Just my few cents,
Mats Kindahl
ramil@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of ram. When ram 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-07-30 16:41:46+05:00, ramil@stripped +7 -0
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
>
> Problem: using "mysqlbinlog | mysql" for recoveries the connection_id()
> result may differ from what was used when issuing the statement.
>
> Fix: if there is a connection_id() in a statement, write to binlog
> SET pseudo_thread_id= XXX; before it and use the value later on.
>
> mysql-test/r/mysqlbinlog.result@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +7
> -0
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - test result.
>
> mysql-test/t/mysqlbinlog.test@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +21
> -0
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - test case.
>
> sql/item_create.cc@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +3 -1
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - set thread_specific_used flag for the connection_id() function.
>
> sql/item_func.cc@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +1 -10
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - always return thd->variables.pseudo_thread_id as a connection_id()
> result, as it contains a proper value for both master and slave.
>
> sql/log_event.cc@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +7 -4
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - set LOG_EVENT_THREAD_SPECIFIC_F event flag if thread_specific_used
> is set.
>
> sql/sql_class.cc@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +1 -1
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - thd->thread_specific_used introduced, which is set if thread specific
> value(s) used in a statement.
>
> sql/sql_class.h@stripped, 2007-07-30 16:41:38+05:00, ramil@stripped +5 -0
> Fix for bug #29928: INSERT ... VALUES(connection_id(), ...) incorrect
> restores from mysqlbinlog out
> - thd->thread_specific_used introduced, which is set if thread specific
> value(s) used in a statement.
>
> diff -Nrup a/mysql-test/r/mysqlbinlog.result b/mysql-test/r/mysqlbinlog.result
> --- a/mysql-test/r/mysqlbinlog.result 2007-06-21 02:11:16 +05:00
> +++ b/mysql-test/r/mysqlbinlog.result 2007-07-30 16:41:38 +05:00
> @@ -318,4 +318,11 @@ INSERT INTO t1 VALUES ('0123456789');
> flush logs;
> DROP TABLE t1;
> # Query thread_id=REMOVED exec_time=REMOVED error_code=REMOVED
> +flush logs;
> +create table t1(a int);
> +insert into t1 values(connection_id());
> +flush logs;
> +drop table t1;
> +1
> +drop table t1;
> End of 5.0 tests
> diff -Nrup a/mysql-test/t/mysqlbinlog.test b/mysql-test/t/mysqlbinlog.test
> --- a/mysql-test/t/mysqlbinlog.test 2007-06-21 02:11:16 +05:00
> +++ b/mysql-test/t/mysqlbinlog.test 2007-07-30 16:41:38 +05:00
> @@ -216,4 +216,25 @@ flush logs;
> DROP TABLE t1;
> --exec $MYSQL_BINLOG --hexdump --local-load=$MYSQLTEST_VARDIR/tmp/
> $MYSQLTEST_VARDIR/log/master-bin.000011 | grep 'Query' | sed 's/[0-9]\{1,\}/REMOVED/g'
>
> +#
> +# Bug #29928: incorrect connection_id() restoring from mysqlbinlog out
> +#
> +flush logs;
> +create table t1(a int);
> +insert into t1 values(connection_id());
> +let $a= `select a from t1`;
> +flush logs;
> +--exec $MYSQL_BINLOG $MYSQLTEST_VARDIR/log/master-bin.000013 >
> $MYSQLTEST_VARDIR/tmp/bug29928.sql
> +drop table t1;
> +connect (con1, localhost, root, , test);
>
> +connection con1;
> +--exec $MYSQL test < $MYSQLTEST_VARDIR/tmp/bug29928.sql
> +--remove_file $MYSQLTEST_VARDIR/tmp/bug29928.sql
> +let $b= `select a from t1`;
> +disconnect con1;
> +connection default;
> +let $c= `select $a=$b`;
> +--echo $c
> +drop table t1;
> +
> --echo End of 5.0 tests
> diff -Nrup a/sql/item_create.cc b/sql/item_create.cc
> --- a/sql/item_create.cc 2007-07-07 22:32:49 +05:00
> +++ b/sql/item_create.cc 2007-07-30 16:41:38 +05:00
> @@ -70,7 +70,9 @@ Item *create_func_ceiling(Item* a)
>
> Item *create_func_connection_id(void)
> {
> - current_thd->lex->safe_to_cache_query= 0;
> + THD *thd= current_thd;
> + thd->lex->safe_to_cache_query= 0;
> + thd->thread_specific_used= 1;
> return new Item_func_connection_id();
> }
>
> diff -Nrup a/sql/item_func.cc b/sql/item_func.cc
> --- a/sql/item_func.cc 2007-06-18 18:45:52 +05:00
> +++ b/sql/item_func.cc 2007-07-30 16:41:38 +05:00
> @@ -649,16 +649,7 @@ bool Item_func_connection_id::fix_fields
> {
> if (Item_int_func::fix_fields(thd, ref))
> return TRUE;
> -
> - /*
> - To replicate CONNECTION_ID() properly we should use
> - pseudo_thread_id on slave, which contains the value of thread_id
> - on master.
> - */
> - value= ((thd->slave_thread) ?
> - thd->variables.pseudo_thread_id :
> - thd->thread_id);
> -
> + value= thd->variables.pseudo_thread_id;
>
Yes. To be able to use the pseudo thread id from a normal client, we
should not make it conditional on being from the slave thread. There
should be no security issue involved, since it is only possible to set
the pseudo_thread_id if you have SUPER privileges.
> return FALSE;
> }
>
> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
> --- a/sql/log_event.cc 2007-06-24 12:58:42 +05:00
> +++ b/sql/log_event.cc 2007-07-30 16:41:38 +05:00
> @@ -1303,8 +1303,9 @@ Query_log_event::Query_log_event(THD* th
> ulong query_length, bool using_trans,
> bool suppress_use, THD::killed_state killed_status_arg)
> :Log_event(thd_arg,
> - ((thd_arg->tmp_table_used ? LOG_EVENT_THREAD_SPECIFIC_F : 0)
> - | (suppress_use ? LOG_EVENT_SUPPRESS_USE_F : 0)),
> + ((thd_arg->tmp_table_used || thd_arg->thread_specific_used) ?
> + LOG_EVENT_THREAD_SPECIFIC_F : 0) |
> + (suppress_use ? LOG_EVENT_SUPPRESS_USE_F : 0),
>
OK. We might be able to merge these two flags, but that should not be
done as a bug-fix. I'll make a note on my refactoring list just to make
sure that it's not lost.
> using_trans),
> data_buf(0), query(query_arg), catalog(thd_arg->catalog),
> db(thd_arg->db), q_len((uint32) query_length),
> @@ -2689,8 +2690,10 @@ Load_log_event::Load_log_event(THD *thd_
> List<Item> &fields_arg,
> enum enum_duplicates handle_dup,
> bool ignore, bool using_trans)
> - :Log_event(thd_arg, !thd_arg->tmp_table_used ?
> - 0 : LOG_EVENT_THREAD_SPECIFIC_F, using_trans),
> + :Log_event(thd_arg,
> + (thd_arg->tmp_table_used || thd_arg->thread_specific_used) ?
> + LOG_EVENT_THREAD_SPECIFIC_F : 0,
> + using_trans),
> thread_id(thd_arg->thread_id),
> slave_proxy_id(thd_arg->variables.pseudo_thread_id),
> num_fields(0),fields(0),
> diff -Nrup a/sql/sql_class.cc b/sql/sql_class.cc
> --- a/sql/sql_class.cc 2007-07-14 17:58:37 +05:00
> +++ b/sql/sql_class.cc 2007-07-30 16:41:38 +05:00
> @@ -197,7 +197,7 @@ THD::THD()
> count_cuted_fields= CHECK_FIELD_IGNORE;
> killed= NOT_KILLED;
> db_length= col_access=0;
> - query_error= tmp_table_used= 0;
> + query_error= tmp_table_used= thread_specific_used= 0;
> next_insert_id=last_insert_id=0;
> hash_clear(&handler_tables_hash);
> tmp_table=0;
> diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
> --- a/sql/sql_class.h 2007-07-20 04:15:46 +05:00
> +++ b/sql/sql_class.h 2007-07-30 16:41:38 +05:00
> @@ -1457,6 +1457,11 @@ public:
> bool in_lock_tables;
> bool query_error, bootstrap, cleanup_done;
> bool tmp_table_used;
> + /*
> + thread_specific_used flag is set if some thread specific value(s)
> + used in a statement.
> + */
> + bool thread_specific_used;
> bool charset_is_system_charset, charset_is_collation_connection;
> bool charset_is_character_set_filesystem;
> bool enable_slow_log; /* enable slow log for current statement */
>
>
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
