Hi, Sanja!
On Feb 17, sa...@mariadb.com wrote:
> revision-id: 83659a68642da2eb6e9327f17a0ef730f512aa96
> parent(s): 3e2849d2a01b06a61407b00989c3f981e62dd183
> committer: Oleksandr Byelkin
> branch nick: server
> timestamp: 2015-02-17 12:54:51 +0100
> message:
>
> MDEV-7006 MDEV-7007: SET STATEMENT and slow log
> fixed embedded server tests
> MDEV-7009: SET STATEMENT min_examined_row_limit has no effect
> MDEV-6948:SET STATEMENT gtid_domain_id = ... FOR has no effect (same for
> gtid_seq_no and server_id)
>
> old values of SET STATENENT variables now saved in its own Query_arena and
> restored later
Looks good to me.
I didn't check, though, whether you've put every restore correctly.
See minor comments below:
> diff --git a/mysql-test/t/set_statement_notembedded_binlog.test
> b/mysql-test/t/set_statement_notembedded_binlog.test
> new file mode 100644
> index 0000000..c9c2334
> --- /dev/null
> +++ b/mysql-test/t/set_statement_notembedded_binlog.test
> @@ -0,0 +1,22 @@
> +--source include/have_log_bin.inc
> +--source include/not_embedded.inc
> +
> +--disable_warnings
> +drop table if exists t1;
> +drop view if exists t1;
> +--enable_warnings
This was useful in our early MySQL days, but it's not anymore.
I've stopped adding these drop...if exists at the beginning of test
files a couple of years ago. mtr has a after-test check to ensure
that no test leaves its tables behind. So one can rely on every test
starting clean.
> +
> +--echo #
> +--echo # MDEV-6948: SET STATEMENT gtid_domain_id = ... FOR has no effect
> +--echo # (same for gtid_seq_no and server_id)
> +--echo #
> +reset master;
> +create table t1 (i int);
> +set gtid_domain_id = 10;
> +insert into t1 values (1),(2);
> +set statement gtid_domain_id = 20 for insert into t1 values (3),(4);
> +
> +--replace_column 1 x 2 x 3 x 4 x 5 x
> +show binlog events limit 5,5;
> +
> +drop table t1;
> diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
> index 0c1c3b3..ee956ed 100644
> --- a/sql/sql_lex.cc
> +++ b/sql/sql_lex.cc
> @@ -4223,6 +4224,56 @@ int LEX::print_explain(select_result_sink *output,
> uint8 explain_flags,
> return res;
> }
>
> +void LEX::set_arena_for_set_stmt(Query_arena *backup)
> +{
> + DBUG_ENTER("LEX::set_arena_for_set_stmt");
> + DBUG_ASSERT(arena_for_set_stmt== 0);
> + if (!mem_root_for_set_stmt)
> + {
> + mem_root_for_set_stmt= new MEM_ROOT();
> + if (!(mem_root_for_set_stmt))
> + DBUG_VOID_RETURN;
What happens in OOM case? You return early, but does the caller know
that? Or it'll continue, thinking that everything's ok?
> + init_sql_alloc(mem_root_for_set_stmt, ALLOC_ROOT_SET, ALLOC_ROOT_SET,
> + MYF(MY_THREAD_SPECIFIC));
> + }
> + if (!(arena_for_set_stmt= new Query_arena(mem_root_for_set_stmt,
Why are you allocating memory for Query_arena, instead of creating it
on mem_root_for_set_stmt?
> + Query_arena::STMT_INITIALIZED)))
> + DBUG_VOID_RETURN;
> + DBUG_PRINT("info", ("mem_root: 0x%lx arena: 0x%lx",
> + (ulong) mem_root_for_set_stmt,
> + (ulong) arena_for_set_stmt));
> + thd->set_n_backup_active_arena(arena_for_set_stmt, backup);
> + DBUG_VOID_RETURN;
> +}
> +
> +
> +void LEX::reset_arena_for_set_stmt(Query_arena *backup)
> +{
> + DBUG_ENTER("LEX::reset_arena_for_set_stmt");
> + DBUG_ASSERT(arena_for_set_stmt);
> + thd->restore_active_arena(arena_for_set_stmt, backup);
> + DBUG_PRINT("info", ("mem_root: 0x%lx arena: 0x%lx",
> + (ulong) arena_for_set_stmt->mem_root,
> + (ulong) arena_for_set_stmt));
> + DBUG_VOID_RETURN;
> +}
> +
> +
> +void LEX::free_arena_for_set_stmt()
> +{
> + DBUG_ENTER("LEX::free_arena_for_set_stmt");
> + if (!arena_for_set_stmt)
> + return;
> + DBUG_PRINT("info", ("mem_root: 0x%lx arena: 0x%lx",
> + (ulong) arena_for_set_stmt->mem_root,
> + (ulong) arena_for_set_stmt));
> + arena_for_set_stmt->free_items();
> + delete(arena_for_set_stmt);
> + free_root(mem_root_for_set_stmt, MYF(MY_KEEP_PREALLOC));
> + arena_for_set_stmt= 0;
> + DBUG_VOID_RETURN;
> +}
> +
> void LEX::restore_set_statement_var()
> {
> DBUG_ENTER("LEX::restore_set_statement_var");
> diff --git a/sql/sql_lex.h b/sql/sql_lex.h
> index 46d667c..ed3c4a0 100644
> --- a/sql/sql_lex.h
> +++ b/sql/sql_lex.h
> @@ -2677,10 +2688,22 @@ struct LEX: public Query_tables_list
> limit_rows_examined_cnt= ULONGLONG_MAX;
> }
>
> +
> + inline void free_set_stmt_mem_root()
> + {
Please, add
DBUG_ASSERT(!is_arena_for_set_stmt());
> + if (mem_root_for_set_stmt)
> + {
> + free_root(mem_root_for_set_stmt, MYF(0));
> + delete mem_root_for_set_stmt;
> + mem_root_for_set_stmt= 0;
> + }
> + }
> +
> LEX();
>
> virtual ~LEX()
> {
> + free_set_stmt_mem_root();
> destroy_query_tables_list();
> plugin_unlock_list(NULL, (plugin_ref *)plugins.buffer, plugins.elements);
> delete_dynamic(&plugins);
Regards,
Sergei
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
