Skip to content

Commit be1851d

Browse files
author
Venkatesh Duggirala
committed
Bug#21229951 VARIABLES IN ALTER EVENT NOT REPLICATED PROPERLY
Problem: SP local variables that are used in ALTER EVENT are not replicated properly. Analysis: CALL statements are not written into binary log. Instead each statement executed in SP is binlogged separately with the exception that we modify query string: we replace uses of SP local variables with NAME_CONST('spvar_name', <spvar-value>) calls. The code was written under the assumption that this replacement was not required for at all for all the queries in 'row' based format. But it can happen that DDLs (which are always binlogged in statement mode irrespective of binlog format) can also use SP local variables and they suffer due to the above assumption. 'ALTER EVENT' in this bug case is one such cases Fix: Any SP local variables used in a query should be replaced with NAME_CONST(...) except for the case when it is DML query and binlog format is 'ROW'.
1 parent 65b36f7 commit be1851d

9 files changed

Lines changed: 77 additions & 17 deletions

File tree

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,20 @@ DROP EVENT event44331_2;
169169
DROP EVENT event44331_3;
170170
DROP EVENT event44331_4;
171171
include/sync_slave_sql_with_master.inc
172+
CREATE EVENT event1 ON SCHEDULE EVERY 100 SECOND STARTS '2000-01-01 00:00:00'
173+
ON COMPLETION PRESERVE ENABLE DO
174+
BEGIN
175+
SET @dummy = 100;
176+
END\\
177+
CREATE PROCEDURE proc1()
178+
BEGIN
179+
DECLARE dummy INT UNSIGNED;
180+
SET dummy = 100;
181+
ALTER EVENT EVENT1 ON SCHEDULE EVERY dummy SECOND
182+
STARTS '2000-01-01 00:00:00' ENABLE;
183+
END \\
184+
CALL proc1();
185+
include/sync_slave_sql_with_master.inc
186+
DROP EVENT event1;
187+
DROP PROCEDURE proc1;
172188
include/rpl_end.inc

‎mysql-test/suite/rpl/t/rpl_events.test‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,32 @@ DROP EVENT event44331_2;
101101
DROP EVENT event44331_3;
102102
DROP EVENT event44331_4;
103103
--source include/sync_slave_sql_with_master.inc
104+
105+
# Bug#21229951 VARIABLES IN ALTER EVENT NOT REPLICATED PROPERLY
106+
connection master;
107+
DELIMITER \\;
108+
CREATE EVENT event1 ON SCHEDULE EVERY 100 SECOND STARTS '2000-01-01 00:00:00'
109+
ON COMPLETION PRESERVE ENABLE DO
110+
BEGIN
111+
SET @dummy = 100;
112+
END\\
113+
114+
# A procedure that has alter event which uses sp local variable in it
115+
CREATE PROCEDURE proc1()
116+
BEGIN
117+
DECLARE dummy INT UNSIGNED;
118+
SET dummy = 100;
119+
ALTER EVENT EVENT1 ON SCHEDULE EVERY dummy SECOND
120+
STARTS '2000-01-01 00:00:00' ENABLE;
121+
END \\
122+
123+
DELIMITER ;\\
124+
125+
CALL proc1();
126+
--source include/sync_slave_sql_with_master.inc
127+
128+
# Cleanup
129+
connection master;
130+
DROP EVENT event1;
131+
DROP PROCEDURE proc1;
104132
--source include/rpl_end.inc

‎sql/binlog.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8112,7 +8112,7 @@ int THD::decide_logging_format(TABLE_LIST *tables)
81128112
my_error((error= ER_BINLOG_ROW_INJECTION_AND_STMT_ENGINE), MYF(0));
81138113
}
81148114
else if (variables.binlog_format == BINLOG_FORMAT_ROW &&
8115-
sqlcom_can_generate_row_events(this))
8115+
sqlcom_can_generate_row_events(this->lex->sql_command))
81168116
{
81178117
/*
81188118
2. Error: Cannot modify table that uses a storage engine
@@ -8151,7 +8151,7 @@ int THD::decide_logging_format(TABLE_LIST *tables)
81518151
my_error((error= ER_BINLOG_ROW_INJECTION_AND_STMT_MODE), MYF(0));
81528152
}
81538153
else if ((flags_write_all_set & HA_BINLOG_STMT_CAPABLE) == 0 &&
8154-
sqlcom_can_generate_row_events(this))
8154+
sqlcom_can_generate_row_events(this->lex->sql_command))
81558155
{
81568156
/*
81578157
5. Error: Cannot modify table that uses a storage engine

‎sql/log_event.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3761,7 +3761,8 @@ Query_log_event::Query_log_event(THD* thd_arg, const char* query_arg,
37613761
cmd_can_generate_row_events= cmd_must_go_to_trx_cache= TRUE;
37623762
break;
37633763
default:
3764-
cmd_can_generate_row_events= sqlcom_can_generate_row_events(thd);
3764+
cmd_can_generate_row_events=
3765+
sqlcom_can_generate_row_events(thd->lex->sql_command);
37653766
break;
37663767
}
37673768
}

‎sql/sp_instr.cc‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ static int cmp_splocal_locations(Item_splocal * const *a,
4545
/*
4646
StoredRoutinesBinlogging
4747
This paragraph applies only to statement-based binlogging. Row-based
48-
binlogging does not need anything special like this.
48+
binlogging does not need anything special like this except for a special
49+
case that is mentioned below in section 2.1
4950
5051
Top-down overview:
5152
@@ -72,6 +73,14 @@ static int cmp_splocal_locations(Item_splocal * const *a,
7273
of SP local variables with NAME_CONST('spvar_name', <spvar-value>) calls.
7374
This substitution is done in subst_spvars().
7475
76+
2.1 Miscellaneous case: DDLs (Eg: ALTER EVENT) in StoredProcedure(SP) uses
77+
its local variables
78+
79+
* Irrespective of binlog format, DDLs are always binlogged in statement mode.
80+
Hence if there are any DDLs, in stored procedure, that uses SP local
81+
variables, those should be replaced with NAME_CONST('spvar_name', <spvar-value>)
82+
even if binlog format is 'row'.
83+
7584
3. FUNCTION calls
7685
7786
In sp_head::execute_function(), we check
@@ -769,7 +778,13 @@ bool sp_instr_stmt::execute(THD *thd, uint *nextp)
769778
770779
- general log is off
771780
772-
- binary logging is off, or not in statement mode
781+
- binary logging is off
782+
783+
- if the query generates row events in binlog row format mode
784+
(DDLs are always written in statement format irrespective of binlog_format
785+
and they can have SP variables in it. For example, 'ALTER EVENT' is allowed
786+
inside a procedure and can contain SP variables in it. Those too need to be
787+
substituted with NAME_CONST(...))
773788
774789
We don't have to substitute on behalf of the query cache as
775790
queries with SP vars are not cached, anyway.
@@ -786,10 +801,11 @@ bool sp_instr_stmt::execute(THD *thd, uint *nextp)
786801
substitution immediately before writing to the log.
787802
*/
788803

789-
need_subst= ((thd->variables.option_bits & OPTION_LOG_OFF) &&
804+
need_subst= !((thd->variables.option_bits & OPTION_LOG_OFF) &&
790805
(!(thd->variables.option_bits & OPTION_BIN_LOG) ||
791806
!mysql_bin_log.is_open() ||
792-
thd->is_current_stmt_binlog_format_row())) ? FALSE : TRUE;
807+
(thd->is_current_stmt_binlog_format_row() &&
808+
sqlcom_can_generate_row_events(m_lex->sql_command))));
793809

794810
/*
795811
If we need to do a substitution but can't (OOM), give up.

‎sql/sp_instr.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by
@@ -350,6 +350,8 @@ class sp_lex_instr : public sp_instr
350350
*/
351351
virtual void cleanup_before_parsing(THD *thd);
352352

353+
/// LEX-object.
354+
LEX *m_lex;
353355
private:
354356
/**
355357
Mem-root for storing the LEX-tree during reparse. This
@@ -358,8 +360,6 @@ class sp_lex_instr : public sp_instr
358360
*/
359361
MEM_ROOT m_lex_mem_root;
360362

361-
/// LEX-object.
362-
LEX *m_lex;
363363

364364
/**
365365
Indicates whether this sp_lex_instr instance is responsible for

‎sql/sql_class.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4259,7 +4259,7 @@ extern "C" bool thd_binlog_filter_ok(const MYSQL_THD thd)
42594259

42604260
extern "C" bool thd_sqlcom_can_generate_row_events(const MYSQL_THD thd)
42614261
{
4262-
return sqlcom_can_generate_row_events(thd);
4262+
return sqlcom_can_generate_row_events(thd->lex->sql_command);
42634263
}
42644264

42654265
extern "C" enum durability_properties thd_get_durability_property(const MYSQL_THD thd)

‎sql/sql_parse.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -599,12 +599,11 @@ void init_update_queries(void)
599599
sql_command_flags[SQLCOM_UNINSTALL_PLUGIN]|= CF_DISALLOW_IN_RO_TRANS;
600600
}
601601

602-
bool sqlcom_can_generate_row_events(const THD *thd)
602+
bool sqlcom_can_generate_row_events(enum enum_sql_command command)
603603
{
604-
return (sql_command_flags[thd->lex->sql_command] &
605-
CF_CAN_GENERATE_ROW_EVENTS);
604+
return (sql_command_flags[command] & CF_CAN_GENERATE_ROW_EVENTS);
606605
}
607-
606+
608607
bool is_update_query(enum enum_sql_command command)
609608
{
610609
DBUG_ASSERT(command >= 0 && command <= SQLCOM_END);

‎sql/sql_parse.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2006, 2012, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2006, 2015, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by
@@ -211,7 +211,7 @@ inline bool is_supported_parser_charset(const CHARSET_INFO *cs)
211211
return (cs->mbminlen == 1);
212212
}
213213

214-
extern "C" bool sqlcom_can_generate_row_events(const THD *thd);
215214

215+
extern "C" bool sqlcom_can_generate_row_events(enum enum_sql_command command);
216216

217217
#endif /* SQL_PARSE_INCLUDED */

0 commit comments

Comments
 (0)