Skip to content

Commit ec2c3bf

Browse files
author
Konstantin Osipov
committed
A pre-requisite patch for the fix for Bug#52044.
This patch also fixes Bug#55452 "SET PASSWORD is replicated twice in RBR mode". The goal of this patch is to remove the release of metadata locks from close_thread_tables(). This is necessary to not mistakenly release the locks in the course of a multi-step operation that involves multiple close_thread_tables() or close_tables_for_reopen(). On the same token, move statement commit outside close_thread_tables(). Other cleanups: Cleanup COM_FIELD_LIST. Don't call close_thread_tables() in COM_SHUTDOWN -- there are no open tables there that can be closed (we leave the locked tables mode in THD destructor, and this close_thread_tables() won't leave it anyway). Make open_and_lock_tables() and open_and_lock_tables_derived() call close_thread_tables() upon failure. Remove the calls to close_thread_tables() that are now unnecessary. Simplify the back off condition in Open_table_context. Streamline metadata lock handling in LOCK TABLES implementation. Add asserts to ensure correct life cycle of statement transaction in a session. Remove a piece of dead code that has also become redundant after the fix for Bug 37521.
1 parent c67cf15 commit ec2c3bf

42 files changed

Lines changed: 589 additions & 467 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎mysql-test/r/variables.result‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,3 +1677,25 @@ SET @@sql_quote_show_create = @sql_quote_show_create_saved;
16771677

16781678
# End of Bug#34828.
16791679

1680+
# Make sure we can manipulate with autocommit in the
1681+
# along with other variables.
1682+
drop table if exists t1;
1683+
drop function if exists t1_max;
1684+
drop function if exists t1_min;
1685+
create table t1 (a int) engine=innodb;
1686+
insert into t1(a) values (0), (1);
1687+
create function t1_max() returns int return (select max(a) from t1);
1688+
create function t1_min() returns int return (select min(a) from t1);
1689+
select t1_min();
1690+
t1_min()
1691+
0
1692+
select t1_max();
1693+
t1_max()
1694+
1
1695+
set @@session.autocommit=t1_min(), @@session.autocommit=t1_max(),
1696+
@@session.autocommit=t1_min(), @@session.autocommit=t1_max(),
1697+
@@session.autocommit=t1_min(), @@session.autocommit=t1_max();
1698+
# Cleanup.
1699+
drop table t1;
1700+
drop function t1_min;
1701+
drop function t1_max;

‎mysql-test/r/view.result‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,15 +1955,15 @@ CHECK TABLE v1, v2, v3, v4, v5, v6;
19551955
Table Op Msg_type Msg_text
19561956
test.v1 check Error FUNCTION test.f1 does not exist
19571957
test.v1 check Error View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
1958-
test.v1 check status Operation failed
1958+
test.v1 check error Corrupt
19591959
test.v2 check status OK
19601960
test.v3 check Error FUNCTION test.f1 does not exist
19611961
test.v3 check Error View 'test.v3' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
1962-
test.v3 check status Operation failed
1962+
test.v3 check error Corrupt
19631963
test.v4 check status OK
19641964
test.v5 check Error FUNCTION test.f1 does not exist
19651965
test.v5 check Error View 'test.v5' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
1966-
test.v5 check status Operation failed
1966+
test.v5 check error Corrupt
19671967
test.v6 check status OK
19681968
create function f1 () returns int return (select max(col1) from t1);
19691969
DROP TABLE t1;

‎mysql-test/suite/rpl/r/rpl_row_implicit_commit_binlog.result‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,6 @@ master-bin.000001 # Table_map # # table_id: # (test.tt_1)
165165
master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
166166
master-bin.000001 # Xid # # COMMIT /* XID */
167167
master-bin.000001 # Query # # use `test`; SET PASSWORD FOR 'user'@'localhost'='*D8DECEC305209EEFEC43008E1D420E1AA06B19E0'
168-
master-bin.000001 # Query # # BEGIN
169-
master-bin.000001 # Table_map # # table_id: # (mysql.user)
170-
master-bin.000001 # Update_rows # # table_id: # flags: STMT_END_F
171-
master-bin.000001 # Query # # COMMIT
172168
-e-e-e-e-e-e-e-e-e-e-e- >> << -e-e-e-e-e-e-e-e-e-e-e-
173169

174170
-b-b-b-b-b-b-b-b-b-b-b- >> << -b-b-b-b-b-b-b-b-b-b-b-

‎mysql-test/t/variables.test‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,4 +1405,30 @@ SET @@sql_quote_show_create = @sql_quote_show_create_saved;
14051405
--echo # End of Bug#34828.
14061406
--echo
14071407

1408+
--echo # Make sure we can manipulate with autocommit in the
1409+
--echo # along with other variables.
1410+
1411+
1412+
--disable_warnings
1413+
drop table if exists t1;
1414+
drop function if exists t1_max;
1415+
drop function if exists t1_min;
1416+
--enable_warnings
1417+
1418+
create table t1 (a int) engine=innodb;
1419+
insert into t1(a) values (0), (1);
1420+
create function t1_max() returns int return (select max(a) from t1);
1421+
create function t1_min() returns int return (select min(a) from t1);
1422+
select t1_min();
1423+
select t1_max();
1424+
set @@session.autocommit=t1_min(), @@session.autocommit=t1_max(),
1425+
@@session.autocommit=t1_min(), @@session.autocommit=t1_max(),
1426+
@@session.autocommit=t1_min(), @@session.autocommit=t1_max();
1427+
1428+
--echo # Cleanup.
1429+
drop table t1;
1430+
drop function t1_min;
1431+
drop function t1_max;
1432+
1433+
14081434
###########################################################################

‎sql/event_data_objects.cc‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,8 @@ Event_job_data::execute(THD *thd, bool drop)
14021402
*/
14031403
thd->set_db(dbname.str, dbname.length);
14041404

1405+
lex_start(thd);
1406+
14051407
#ifndef NO_EMBEDDED_ACCESS_CHECKS
14061408
if (event_sctx.change_security_context(thd,
14071409
&definer_user, &definer_host,
@@ -1411,7 +1413,7 @@ Event_job_data::execute(THD *thd, bool drop)
14111413
"[%s].[%s.%s] execution failed, "
14121414
"failed to authenticate the user.",
14131415
definer.str, dbname.str, name.str);
1414-
goto end_no_lex_start;
1416+
goto end;
14151417
}
14161418
#endif
14171419

@@ -1427,11 +1429,11 @@ Event_job_data::execute(THD *thd, bool drop)
14271429
"[%s].[%s.%s] execution failed, "
14281430
"user no longer has EVENT privilege.",
14291431
definer.str, dbname.str, name.str);
1430-
goto end_no_lex_start;
1432+
goto end;
14311433
}
14321434

14331435
if (construct_sp_sql(thd, &sp_sql))
1434-
goto end_no_lex_start;
1436+
goto end;
14351437

14361438
/*
14371439
Set up global thread attributes to reflect the properties of
@@ -1451,8 +1453,6 @@ Event_job_data::execute(THD *thd, bool drop)
14511453
if (parser_state.init(thd, thd->query(), thd->query_length()))
14521454
goto end;
14531455

1454-
lex_start(thd);
1455-
14561456
if (parse_sql(thd, & parser_state, creation_ctx))
14571457
{
14581458
sql_print_error("Event Scheduler: "
@@ -1484,13 +1484,6 @@ Event_job_data::execute(THD *thd, bool drop)
14841484
}
14851485

14861486
end:
1487-
if (thd->lex->sphead) /* NULL only if a parse error */
1488-
{
1489-
delete thd->lex->sphead;
1490-
thd->lex->sphead= NULL;
1491-
}
1492-
1493-
end_no_lex_start:
14941487
if (drop && !thd->is_fatal_error)
14951488
{
14961489
/*
@@ -1529,7 +1522,6 @@ Event_job_data::execute(THD *thd, bool drop)
15291522
if (save_sctx)
15301523
event_sctx.restore_security_context(thd, save_sctx);
15311524
#endif
1532-
lex_end(thd->lex);
15331525
thd->lex->unit.cleanup();
15341526
thd->end_statement();
15351527
thd->cleanup_after_query();

‎sql/event_db_repository.cc‎

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -518,17 +518,20 @@ Event_db_repository::table_scan_all_for_i_s(THD *thd, TABLE *schema_table,
518518
*/
519519

520520
bool
521-
Event_db_repository::fill_schema_events(THD *thd, TABLE_LIST *tables,
521+
Event_db_repository::fill_schema_events(THD *thd, TABLE_LIST *i_s_table,
522522
const char *db)
523523
{
524-
TABLE *schema_table= tables->table;
525-
TABLE *event_table= NULL;
524+
TABLE *schema_table= i_s_table->table;
525+
Open_tables_backup open_tables_backup;
526+
TABLE_LIST event_table;
526527
int ret= 0;
527528

528529
DBUG_ENTER("Event_db_repository::fill_schema_events");
529530
DBUG_PRINT("info",("db=%s", db? db:"(null)"));
530531

531-
if (open_event_table(thd, TL_READ, &event_table))
532+
event_table.init_one_table("mysql", 5, "event", 5, "event", TL_READ);
533+
534+
if (open_system_tables_for_read(thd, &event_table, &open_tables_backup))
532535
DBUG_RETURN(TRUE);
533536

534537
/*
@@ -541,11 +544,11 @@ Event_db_repository::fill_schema_events(THD *thd, TABLE_LIST *tables,
541544
every single row's `db` with the schema which we show.
542545
*/
543546
if (db)
544-
ret= index_read_for_db_for_i_s(thd, schema_table, event_table, db);
547+
ret= index_read_for_db_for_i_s(thd, schema_table, event_table.table, db);
545548
else
546-
ret= table_scan_all_for_i_s(thd, schema_table, event_table);
549+
ret= table_scan_all_for_i_s(thd, schema_table, event_table.table);
547550

548-
close_thread_tables(thd);
551+
close_system_tables(thd, &open_tables_backup);
549552

550553
DBUG_PRINT("info", ("Return code=%d", ret));
551554
DBUG_RETURN(ret);
@@ -584,10 +587,7 @@ Event_db_repository::open_event_table(THD *thd, enum thr_lock_type lock_type,
584587
tables.init_one_table("mysql", 5, "event", 5, "event", lock_type);
585588

586589
if (open_and_lock_tables(thd, &tables, FALSE, MYSQL_LOCK_IGNORE_TIMEOUT))
587-
{
588-
close_thread_tables(thd);
589590
DBUG_RETURN(TRUE);
590-
}
591591

592592
*table= tables.table;
593593
tables.table->use_all_columns();
@@ -700,7 +700,8 @@ Event_db_repository::create_event(THD *thd, Event_parse_data *parse_data,
700700

701701
end:
702702
if (table)
703-
close_thread_tables(thd);
703+
close_mysql_tables(thd);
704+
704705
thd->variables.sql_mode= saved_mode;
705706
DBUG_RETURN(test(ret));
706707
}
@@ -811,7 +812,8 @@ Event_db_repository::update_event(THD *thd, Event_parse_data *parse_data,
811812

812813
end:
813814
if (table)
814-
close_thread_tables(thd);
815+
close_mysql_tables(thd);
816+
815817
thd->variables.sql_mode= saved_mode;
816818
DBUG_RETURN(test(ret));
817819
}
@@ -865,7 +867,7 @@ Event_db_repository::drop_event(THD *thd, LEX_STRING db, LEX_STRING name,
865867

866868
end:
867869
if (table)
868-
close_thread_tables(thd);
870+
close_mysql_tables(thd);
869871

870872
DBUG_RETURN(test(ret));
871873
}
@@ -933,34 +935,14 @@ Event_db_repository::find_named_event(LEX_STRING db, LEX_STRING name,
933935

934936
void
935937
Event_db_repository::drop_schema_events(THD *thd, LEX_STRING schema)
936-
{
937-
DBUG_ENTER("Event_db_repository::drop_schema_events");
938-
drop_events_by_field(thd, ET_FIELD_DB, schema);
939-
DBUG_VOID_RETURN;
940-
}
941-
942-
943-
/**
944-
Drops all events which have a specific value of a field.
945-
946-
@pre The thread handle has no open tables.
947-
948-
@param[in,out] thd Thread
949-
@param[in,out] table mysql.event TABLE
950-
@param[in] field Which field of the row to use for matching
951-
@param[in] field_value The value that should match
952-
*/
953-
954-
void
955-
Event_db_repository::drop_events_by_field(THD *thd,
956-
enum enum_events_table_field field,
957-
LEX_STRING field_value)
958938
{
959939
int ret= 0;
960940
TABLE *table= NULL;
961941
READ_RECORD read_record_info;
962-
DBUG_ENTER("Event_db_repository::drop_events_by_field");
963-
DBUG_PRINT("enter", ("field=%d field_value=%s", field, field_value.str));
942+
enum enum_events_table_field field= ET_FIELD_DB;
943+
MDL_ticket *mdl_savepoint= thd->mdl_context.mdl_savepoint();
944+
DBUG_ENTER("Event_db_repository::drop_schema_events");
945+
DBUG_PRINT("enter", ("field=%d schema=%s", field, schema.str));
964946

965947
if (open_event_table(thd, TL_WRITE, &table))
966948
DBUG_VOID_RETURN;
@@ -979,7 +961,7 @@ Event_db_repository::drop_events_by_field(THD *thd,
979961
get_field(thd->mem_root,
980962
table->field[ET_FIELD_NAME])));
981963

982-
if (!sortcmp_lex_string(et_field_lex, field_value, system_charset_info))
964+
if (!sortcmp_lex_string(et_field_lex, schema, system_charset_info))
983965
{
984966
DBUG_PRINT("info", ("Dropping"));
985967
if ((ret= table->file->ha_delete_row(table->record[0])))
@@ -989,6 +971,11 @@ Event_db_repository::drop_events_by_field(THD *thd,
989971
}
990972
end_read_record(&read_record_info);
991973
close_thread_tables(thd);
974+
/*
975+
Make sure to only release the MDL lock on mysql.event, not other
976+
metadata locks DROP DATABASE might have acquired.
977+
*/
978+
thd->mdl_context.rollback_to_savepoint(mdl_savepoint);
992979

993980
DBUG_VOID_RETURN;
994981
}
@@ -1026,7 +1013,7 @@ Event_db_repository::load_named_event(THD *thd, LEX_STRING dbname,
10261013
else if ((ret= etn->load_from_row(thd, table)))
10271014
my_error(ER_CANNOT_LOAD_FROM_TABLE, MYF(0), "event");
10281015

1029-
close_thread_tables(thd);
1016+
close_mysql_tables(thd);
10301017
}
10311018

10321019
thd->variables.sql_mode= saved_mode;
@@ -1104,7 +1091,8 @@ update_timing_fields_for_event(THD *thd,
11041091

11051092
end:
11061093
if (table)
1107-
close_thread_tables(thd);
1094+
close_mysql_tables(thd);
1095+
11081096
/* Restore the state of binlog format */
11091097
DBUG_ASSERT(!thd->is_current_stmt_binlog_format_row());
11101098
if (save_binlog_row_based)
@@ -1151,7 +1139,7 @@ Event_db_repository::check_system_tables(THD *thd)
11511139
if (table_intact.check(tables.table, &mysql_db_table_def))
11521140
ret= 1;
11531141

1154-
close_thread_tables(thd);
1142+
close_mysql_tables(thd);
11551143
}
11561144
/* Check mysql.user */
11571145
tables.init_one_table("mysql", 5, "user", 4, "user", TL_READ);
@@ -1171,7 +1159,7 @@ Event_db_repository::check_system_tables(THD *thd)
11711159
event_priv_column_position);
11721160
ret= 1;
11731161
}
1174-
close_thread_tables(thd);
1162+
close_mysql_tables(thd);
11751163
}
11761164
/* Check mysql.event */
11771165
tables.init_one_table("mysql", 5, "event", 5, "event", TL_READ);
@@ -1185,7 +1173,7 @@ Event_db_repository::check_system_tables(THD *thd)
11851173
{
11861174
if (table_intact.check(tables.table, &event_table_def))
11871175
ret= 1;
1188-
close_thread_tables(thd);
1176+
close_mysql_tables(thd);
11891177
}
11901178

11911179
DBUG_RETURN(test(ret));

‎sql/event_db_repository.h‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class Event_db_repository
9191
bool
9292
load_named_event(THD *thd, LEX_STRING dbname, LEX_STRING name, Event_basic *et);
9393

94-
bool
94+
static bool
9595
open_event_table(THD *thd, enum thr_lock_type lock_type, TABLE **table);
9696

9797
bool
@@ -109,9 +109,6 @@ class Event_db_repository
109109
static bool
110110
check_system_tables(THD *thd);
111111
private:
112-
void
113-
drop_events_by_field(THD *thd, enum enum_events_table_field field,
114-
LEX_STRING field_value);
115112
bool
116113
index_read_for_db_for_i_s(THD *thd, TABLE *schema_table, TABLE *event_table,
117114
const char *db);

‎sql/events.cc‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
#include "sql_priv.h"
1717
#include "unireg.h"
1818
#include "sql_parse.h" // check_access
19-
#include "sql_base.h" // close_thread_tables
19+
#include "sql_base.h" // close_mysql_tables
2020
#include "sql_show.h" // append_definer
2121
#include "events.h"
2222
#include "sql_db.h" // check_db_dir_existence
@@ -754,7 +754,6 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
754754
{
755755
char *db= NULL;
756756
int ret;
757-
Open_tables_backup open_tables_backup;
758757
DBUG_ENTER("Events::fill_schema_events");
759758

760759
if (check_if_system_tables_error())
@@ -773,15 +772,7 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
773772
DBUG_RETURN(1);
774773
db= thd->lex->select_lex.db;
775774
}
776-
/*
777-
Reset and backup of the currently open tables in this thread
778-
is a way to allow SELECTs from INFORMATION_SCHEMA.events under
779-
LOCK TABLES and in pre-locked mode. See also
780-
Events::show_create_event for additional comments.
781-
*/
782-
thd->reset_n_backup_open_tables_state(&open_tables_backup);
783775
ret= db_repository->fill_schema_events(thd, tables, db);
784-
thd->restore_backup_open_tables_state(&open_tables_backup);
785776

786777
DBUG_RETURN(ret);
787778
}
@@ -1161,8 +1152,7 @@ Events::load_events_from_db(THD *thd)
11611152
end:
11621153
end_read_record(&read_record_info);
11631154

1164-
close_thread_tables(thd);
1165-
1155+
close_mysql_tables(thd);
11661156
DBUG_RETURN(ret);
11671157
}
11681158

0 commit comments

Comments
 (0)