Hello Ramil,
> 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.
> 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;
maybe use "true" or "TRUE", as this is a bool variable.
> return new Item_func_connection_id();
> }
> 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),
> 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),
THD::tmp_table_used is set when the thread uses a tmp table, and is
tested only in two places above.
So, it is easily possible to remove tmp_table_used from THD, and use
thread_specific_used instead.
Mats already pointed this out but suggested to leave it as a cleanup
for later.
I would prefer to do it now, as it looks 100% safe, and thus the code
would be simpler and your patch wouldn't even make THD grow.
But 5.0 is sacred, so I would suggest to do it only in 5.1 when you
merge your patch into 5.1; the grep shows:
cd /m/mysql-5.1/sql/
grep -n tmp_table_used *.h *.c *.cc /dev/null
sql_class.h:1398: bool tmp_table_used;
log_event.cc:1483: ((thd_arg->tmp_table_used ? LOG_EVENT_THREAD_SPECIFIC_F : 0)
log_event.cc:2931: :Log_event(thd_arg, !thd_arg->tmp_table_used ?
sql_base.cc:2278: thd->tmp_table_used= 1;
sql_class.cc:367: query_error= tmp_table_used= 0;
sql_parse.cc:5146: thd->tmp_table_used= 0;
sql_table.cc:1702: thd->tmp_table_used= tmp_table_deleted;
sql_table.cc:3451: thd->tmp_table_used= 1;
The line
sql_table.cc:1702: thd->tmp_table_used= tmp_table_deleted;
should then be changed to
thd->thread_specific_used|= tmp_table_deleted
(the "or" is to not reset the effect of connection_id(), in the case
the statement used connection_id() too, though this is impossible in
practice as sql_table.cc:1702 is about DROP TABLE which cannot use
connection_id()).
If you prefer to not do it (but I encourage you to do it :), please at
least put a
/** @todo etc */
comment near "tmp_table_used" in "class THD", though
I fear that as many todos it never gets done.
> 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;
Please make the comment into doxygen style, then it will be
automatically be shown near thread_specific_used when one browses the
doxygen output.
Ok to push.
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Mr. Guilhem Bichot <guilhem@stripped>
/ /|_/ / // /\ \/ /_/ / /__ MySQL AB, Lead Software Engineer
/_/ /_/\_, /___/\___\_\___/ Bordeaux, France
<___/ www.mysql.com
