Hi Libing,
Thank you for the work, I have some comments!
STATUS
------
Not Approved!
REQUESTS
--------
R1. When there is error when applying the rows event (e.g. cannot open
table, duplicate key error, etc), then it seems it will not call
rli->context_cleanup(), and so the thd->options will not be restored in
these cases. Please add code and test cases to handle this.
R2. I think the test case should not be added to
binlog_base64_flag.test, which is used for testing --base64-output flags
of mysqlbinlog client tool, I'd suggest use binlog_row_binlog.test, or
create a new test.
Li-Bing.Song@stripped wrote:
> #At file:///home/anders/Work/bzrwork/wt1/mysql-5.1-bugteam/ based on
> revid:anitha.gopi@stripped
>
> 3548 Li-Bing.Song@stripped 2010-11-25
> BUG#54903 BINLOG statement toggles session variables
>
> When using BINLOG statement to execute rows log events, session variables
> foreign_key_checks and unique_checks are changed temporarily. As each rows
> log event has their own special session environment and its own
> foreign_key_checks and unique_checks can be different from current session
> which executing the BINLOG statement. But these variables are not restored
> correctly after BINLOG statement. This problem will cause that the following
> statements fail or generate unexpected data.
>
> In this patch, code is added to backup and restore these two variables.
> So BINLOG statement will not affect current session's variables again.
> @ mysql-test/suite/binlog/t/binlog_base64_flag.test
> Add a test to verify this patch.
> @ sql/log_event.cc
> Backup thd->options before changing it.
> @ sql/rpl_rli.cc
> Initialze added variable in Relay_log_info constructor,
> and restore thd->options in cleanup_context function.
> @ sql/rpl_rli.h
> Add variables and backup function for Rows log events.
>
> modified:
> mysql-test/suite/binlog/r/binlog_base64_flag.result
> mysql-test/suite/binlog/t/binlog_base64_flag.test
> sql/log_event.cc
> sql/rpl_rli.cc
> sql/rpl_rli.h
> === modified file 'mysql-test/suite/binlog/r/binlog_base64_flag.result'
> --- a/mysql-test/suite/binlog/r/binlog_base64_flag.result 2010-06-18 17:32:23 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_base64_flag.result 2010-11-25 06:56:55 +0000
> @@ -102,3 +102,45 @@ BINLOG '-2079193929';
> ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds
> to your MySQL server version for the right syntax to use
> BINLOG 'xç↓%~∙D╒ƒ╡';
> ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds
> to your MySQL server version for the right syntax to use
> +
> +# BUG#54903 BINLOG statement toggles session variables
> +# ----------------------------------------------------------------------
> +# This test verify that BINLOG statement doesn't change current session's
> +# variables foreign_key_checks and unique_checks.
> +
> +CREATE TABLE t1 (c1 INT);
> +SET @@SESSION.foreign_key_checks= ON;
> +SET @@SESSION.unique_checks= ON;
> +# INSERT INTO t1 VALUES (1)
> +# foreign_key_checks=0 and unique_checks=0
> +BINLOG '
> +dfLtTBMBAAAAKQAAANcAAAAAABcAAAAAAAEABHRlc3QAAnQxAAEDAAE=
> +dfLtTBcBAAAAIgAAAPkAAAAAABcAAAAAAAcAAf/+AQAAAA==
> +';
> +SELECT * FROM t1;
> +c1
> +1
> +# Their values should be ON
> +SHOW SESSION VARIABLES LIKE "%_checks";
> +Variable_name Value
> +foreign_key_checks ON
> +unique_checks ON
> +
> +SET @@SESSION.foreign_key_checks= OFF;
> +SET @@SESSION.unique_checks= OFF;
> +# INSERT INTO t1 VALUES(2)
> +# foreign_key_checks=1 and unique_checks=1
> +BINLOG '
> +dfLtTBMBAAAAKQAAAKsBAAAAABcAAAAAAAEABHRlc3QAAnQxAAEDAAE=
> +dfLtTBcBAAAAIgAAAM0BAAAAABcAAAAAAAEAAf/+AgAAAA==
> +';
> +SELECT * FROM t1;
> +c1
> +1
> +2
> +# Their values should be OFF
> +SHOW SESSION VARIABLES LIKE "%_checks";
> +Variable_name Value
> +foreign_key_checks OFF
> +unique_checks OFF
> +DROP TABLE t1;
>
> === modified file 'mysql-test/suite/binlog/t/binlog_base64_flag.test'
> --- a/mysql-test/suite/binlog/t/binlog_base64_flag.test 2010-06-18 17:32:23 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_base64_flag.test 2010-11-25 06:56:55 +0000
> @@ -163,3 +163,50 @@ BINLOG '123';
> BINLOG '-2079193929';
> --error ER_SYNTAX_ERROR
> BINLOG 'xç↓%~∙D╒ƒ╡';
> +
> +--echo
> +--echo # BUG#54903 BINLOG statement toggles session variables
> +--echo # ----------------------------------------------------------------------
> +--echo # This test verify that BINLOG statement doesn't change current session's
> +--echo # variables foreign_key_checks and unique_checks.
> +--echo
> +CREATE TABLE t1 (c1 INT);
> +
> +SET @@SESSION.foreign_key_checks= ON;
> +SET @@SESSION.unique_checks= ON;
> +
> +--echo # INSERT INTO t1 VALUES (1)
> +--echo # foreign_key_checks=0 and unique_checks=0
> +BINLOG '
> +dfLtTBMBAAAAKQAAANcAAAAAABcAAAAAAAEABHRlc3QAAnQxAAEDAAE=
> +dfLtTBcBAAAAIgAAAPkAAAAAABcAAAAAAAcAAf/+AQAAAA==
> +';
> +
> +SELECT * FROM t1;
> +--echo # Their values should be ON
> +SHOW SESSION VARIABLES LIKE "%_checks";
> +
> +--echo
> +SET @@SESSION.foreign_key_checks= OFF;
> +SET @@SESSION.unique_checks= OFF;
> +
> +--echo # INSERT INTO t1 VALUES(2)
> +--echo # foreign_key_checks=1 and unique_checks=1
> +BINLOG '
> +dfLtTBMBAAAAKQAAAKsBAAAAABcAAAAAAAEABHRlc3QAAnQxAAEDAAE=
> +dfLtTBcBAAAAIgAAAM0BAAAAABcAAAAAAAEAAf/+AgAAAA==
> +';
> +
> +SELECT * FROM t1;
> +--echo # Their values should be OFF
> +SHOW SESSION VARIABLES LIKE "%_checks";
> +
> +BINLOG '
> +QQTuTBMBAAAAKQAAANcAAAAAABcAAAAAAAEABHRlc3QAAnQxAAEDAAE=
> +QQTuTBcBAAAAIgAAAPkAAAAAABcAAAAAAAcAAf/+AQAAAA==
> +';
> +BINLOG '
> +QQTuTAIBAAAARQAAAD4BAAAIAAMAAAAAAAAABAAAGgAAAEAADAEAAAAAAAAAAAYDc3RkBAgACAAI
> +AHRlc3QAQ09NTUlU
> +';
> +DROP TABLE t1;
> \ No newline at end of file
>
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc 2010-10-23 12:55:44 +0000
> +++ b/sql/log_event.cc 2010-11-25 06:56:55 +0000
> @@ -7425,6 +7425,7 @@ int Rows_log_event::do_apply_event(Relay
> Make sure to set/clear them before executing the main body of
> the event.
> */
> + const_cast<Relay_log_info *>(rli)->backup_thd_options(thd);
> if (get_flags(NO_FOREIGN_KEY_CHECKS_F))
> thd->options|= OPTION_NO_FOREIGN_KEY_CHECKS;
> else
>
> === modified file 'sql/rpl_rli.cc'
> --- a/sql/rpl_rli.cc 2010-07-02 18:30:47 +0000
> +++ b/sql/rpl_rli.cc 2010-11-25 06:56:55 +0000
> @@ -43,7 +43,8 @@ Relay_log_info::Relay_log_info()
> inited(0), abort_slave(0), slave_running(0), until_condition(UNTIL_NONE),
> until_log_pos(0), retried_trans(0),
> tables_to_lock(0), tables_to_lock_count(0),
> - last_event_start_time(0), m_flags(0)
> + last_event_start_time(0), m_flags(0),
> + thd_options(0), thd_options_inited(FALSE)
> {
> DBUG_ENTER("Relay_log_info::Relay_log_info");
>
> @@ -1210,11 +1211,12 @@ void Relay_log_info::cleanup_context(THD
> close_thread_tables(thd);
> clear_tables_to_lock();
> clear_flag(IN_STMT);
> - /*
> - Cleanup for the flags that have been set at do_apply_event.
> - */
> - thd->options&= ~OPTION_NO_FOREIGN_KEY_CHECKS;
> - thd->options&= ~OPTION_RELAXED_UNIQUE_CHECKS;
> +
> + if (thd_options_inited)
> + {
> + thd->options= thd_options;
> + thd_options_inited= FALSE;
> + }
> last_event_start_time= 0;
> DBUG_VOID_RETURN;
> }
>
> === modified file 'sql/rpl_rli.h'
> --- a/sql/rpl_rli.h 2010-09-24 09:44:53 +0000
> +++ b/sql/rpl_rli.h 2010-11-25 06:56:55 +0000
> @@ -423,8 +423,18 @@ public:
> (m_flags & (1UL << IN_STMT));
> }
>
> + void backup_thd_options(const THD *thd)
> + {
> + thd_options= thd->options;
> + thd_options_inited= TRUE;
> + }
> private:
> uint32 m_flags;
> + /*
> + Used to backup and restore current session's options for BINLOG statement.
> + */
> + ulonglong thd_options;
> + bool thd_options_inited;
> };
>
>
>
> text/bzr-bundle 类型 附件
> (bzr/li-bing.song@stripped)
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: li-bing.song@stripped
> # target_branch: file:///home/anders/Work/bzrwork/wt1/mysql-5.1-\
> # bugteam/
> # testament_sha1: 9b943c20a773d3b2ddcf05dc056cd1c787635fd6
> # timestamp: 2010-11-25 14:57:00 +0800
> # base_revision_id: anitha.gopi@stripped\
> # h9gdxjzhjv8zxse9
> #
> # Begin bundle
> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWXkoNZAABmZfgHqwWv///3/v
> /6C////6YA1G277ydz7rWifI2YH3fXXVnxvuvqxo+hA7UYlX3328Dx8JJImmSanqPNUeyTGmqb01
> T8p5U0ekZqaeo0NA09RkyAeoJKBBPUxHqZMlHqeU2j1QA9RoABoAA0AAlBBpTVP1R+UjJ6hoAGI0
> HqBpkAPU0yBmiPUCRIiJM2im0yY9KhtTTQaeUZHqNA0NAHqAAaCKSACDUwmE00U8TKeTKbTQmmjQ
> DEBo0ACSIhoATQaAQGhqNTU2o9TyjTQ0APUD1BppSARHQwOEjsB439FCMgx52gzXIOCarcw0OV4d
> 17er8HZ0l+uzTg3F8gqx2v79j0Y+S0OIt7jgINS97EN9n5HF4iCdhMmVvW9fHr5Las3g6xe/qm22
> YsOTYZDs0F1HH3sSTT4NApCkJtJHjlxGn7Y3TaKslu1sWOJoZIfO66qZCusJhGFhwwtgSiPUL4mF
> dXzCGD2ax7WjhRRDbbGxtN3/qjRPadrbhhMduWJNUrjK50hxmg6ImIF1Zu8Fl/rm2k39ntCAaSwN
> wSJwYYNa3CHVN8wpcF6nMDsB3e3fGlxvIMGUgY6BAmL9FdFi4ntRmkk7MxFemleOVBFmpiGjG2K5
> VwvyWW94xL/FGNEXakih1QqYCvohROpQBwv8xG0+YN8xmLPpQWiNBbobfobLDF8aqHxqtIIjU0AJ
> sgdwuuf8BjhGhkecFeTTVVzQ2Y2vDGfhhYgngR5bXreLCGdBuhdhJHcm222NsY2x9ZbwQKQqCuZn
> M4S2CT1RHBMpRu3bWPRW+TYSaNXVti86j2hhQumq02o1O4U43xBvuyBcZQDcmZ0To8d9jcLtm295
> UuxWmTkYi6TLLROtnpW5ZKBbvaaaNvnqIAwNWTYQodoXtKVNkAk4d/FwJCWXW5JkvZxsz9BFaSxP
> DDhDiIamZMfsU8iziqTQ8RSJAZUxKBSudLquMoCm9kNMYmlqCxqapJsFlJaaGCj+0xBKC+omssLX
> KSI06Sa7toGcMS3IgphwiT8B39VgfNCJ7oXubtmww5eYXM5mwW6yqvXsScwa3VAtujLK/XZvrnjD
> uFypac9821dOCTd2arFQ4kyP857+QrqnZi6R5tuZJ9pVsis6nY3xG8dC2nASM6ZllRxiJcSY+v/l
> Wbau5r4Ptc5dppQBnrCSMZD1tFVOuwzmOuWVEiSyKN2KssPaoFCAwVZNFSTrRDtL6K2Z7KDWaoAc
> wcQUMtSqUlD/Dk3hXTiJbX0phGeTNOqCmWLRqZjMaiNSsfL+1qYrDAYwjcfyA9hkyTQilJYHeWED
> ZBdOC/Aqx8F0WsyGV5kGQ4GUzoY5aYMUKA7MNeadkKMmpDZpUSaafOBog5Zd5lCmoqIFKls88Y7X
> fxDAUP0YATjRUcjLUb1WcFyis164GszY6Xp2OcXfhQLwz0OxOaktneNvKtx6XVbkVxQaJPJD6zOx
> VorGGWjAyRTlVZrHF3VW9cCzLFZ7M1IwijSbtzedbeBSSEp6BKedWk2MGlyrDDDNVm7Ci1KnAkdp
> e5iVm01E5abjmeGpSWMhrnLq42rTMmRGE8lvTg2L1GpaliWalkUUM1ReVB16yNkpkqjAqrahzrko
> LiaDmu7xmUDHVI04Xp3FqGNxEwg47E5I0EYbyE+mo0mZTfYNl+CjraocDAMFG+JNUQ5WG+I1JSw6
> fAiSUqjQVkiDY7anhvncrNK5LKymcHHLi4Iju73hfqaehNFEBNNO6UQdyrOJhesRJ3AtZevm91z+
> 89Gouym4REJUKOSIJne3+UfBLUvvAiCjU1xBDB1pRMwJQB7Uu9Kc1U7B4gO0DgIsHFamSXSIEm2J
> q0lSrEwjZJTTSSJEMNU/nSXypHwhPZA2tJWkJ6woCylaWS5MTMdp94Ogp+igFR3EyIl7o2ZnMLEM
> EGVMyYdpqPf49V6WDeJIDOmWYjD9D9YIShMKVgqHI7IK5p/gQf0PsfuqJoRNJ0qBJCgFBwgkCcYH
> 5ZPeBAQAUNV1Qbkmbgi0JgCBCYJtoYa2uDoQVDngoQGYAkJp+IGAOp0OCKRC+UVgH31DMGABiEID
> UAnCBtCkklDNH2CyXkvgj2FAbg6mzYGjXhZgJ5gMi4N0JhxASBZgVYKPlgCQG4F0uhr4xtDeElMG
> 2liXeuWBasO5cAoYDMD/vGBvmB620M/wMIGMYxjNdSKtIHJWGeaIRUcSJiUgIFN5EJz1RDPCCSuG
> xWSZzYYEzmcPTem4b4w89BmPafVpyIXGe+iTVwMOW0hzR44vAZgiYBypz32H3H9AezilScNj2rsZ
> BvD4qW6JqQv623DCKvdbG4Cu2T0zDKX5KUhwQo0T2qREwmgbLRu1521YnEXhkDthNwFHWXEUE7oL
> hlXuSW1j6smdeI1Gp2EYqO6kMSrHFJNbqA0A85Ci0gYEn3axeS/GstTLM2lSJ/E9yKcR0JkRkuVt
> E5WBvDDvRkMGgoJR2lq+jQtC4AKxKeLgA6UZDWJI4c/fGntrqLBsJ0OXer6J8OwIkAZMA0WPYLWj
> 1Njl3jEo5FYZsq0mrVlKUxNEtGU6RakUlxfqnIxOTlQx3HxPvxFsFLlWwmxz1k5ImcWgP3i0KdJU
> dBT7zWX5w9vmUDQ3oma2PPk4ztVMQZEvZSZHiWqYjpNxr0dlWELVTAvAfkXEFkjFEVpaTDuYCWhB
> SAxvOnM25yn+sy1FnqhcJABe+/5jV3YadZEiNrGTHYRMWffomyYbI/B4+jAWlMdemMvll6jZSDVo
> 3NvhCSSIZIdDA7DpcZ9Tc7+G6S4WguYNX4XnaXopEWpO06L+omHAYuiCzEp6r+Il5IfdtPA2qcgH
> Q7jaI6GHWf3+z0iSKi4C4wFW3Bden3Is80MGCOMuCDmt/ouIULbUDnuBhV96f0EtHCaJcuJ7cS0Z
> QFbCsJLtwJwDkP49iSpQKrHuaiZGL6GXYxiF3OZq6+kzNTT2C+IriGDaHwKtDpCih7AZBhMKapgA
> sm9Yf3I0wTNguJ4CtHqvDvdDvBtd31QryQJMcvb1PyI4oo63yTbuXS6OpXfnFmt4sa+O5yi8ryuz
> yF4JjKrm5prA1ZDHXk57PRoVqSFPR2891hXCEQsAHCLnTiw1p0zATSaEJaa0Z4iES41mWqyQoVyk
> zJqVNyrRqO4rU+qTvLOrxF/F8Na7BKk6pwIFQkF62E2GzYI4htA2KpSivjSbbRGJBrJrpGKcKIkS
> t32EnDmEVJsSeJoPWfITIeAjZSdM7HvPFBV4KvgRMJZ6k+JC4WHt39ujmp5ss3sgJDhF8tQQJMjC
> SRYiJE41LvJKr8ankKm6ddiwNi76D+BA9bMCxnIfltgBWNM2YIbmmR6EjaLxGpODFWQjYBVdPMs0
> IEAE4Rc46BaEocUlKbC5tZlCxtFZQ0wJXJlp0ox7dJKOCNG51tGIxRteWXSeBvuOREh7sTnvstAc
> wImlaksXnmAJw4ZRNa+zNuAMwzItIYcmA84uDICuEFjku87yFwY1l55Zm/17QbjtSC6EVT4MiFgg
> KRppxVsNwlGFkgsstqAOYvcjllg3Z2M+AixMVLnQt4l6W9zEO9gf5HDvEp1FBkdjwsXntnIg1QrR
> JJheELrOyYZ0ab4ZZXwnE1XMORV8K8Biz0x9CdhpotMQos6iL23LaavBI/Tb0WQTQ8W8UPIWhWVU
> miKWlAbUJVyrV2LqJQBRQKcI5E+5HCQsYVAdFdnZpEXNFghwdgSpiIkBBF4OBqxWHbXfW+AWotbd
> +IiIiIi/gWoJJmwks1rdGt2dYSEeEJAME9c5D6gRNpU67SpQrjyOLre0WRPtehkw2p6RaVdMG1kY
> TIn0Au7x15q8nvv+QECxGIhnPKd0JlO1/VUOd+pyhljQkYvydasdY98REOkgpUV9PR18G3DDtBnG
> S9VRslrlxNmktANR7lDraHAZQzBaQZrWLWSehT57VavzrypzPgveJOHJrlrCuyCEOP0GDiDB6hoY
> nyRum9UpjU+Dl1NSvQ/Gt7BM+JtnnAn52lXG9ex0HbyHedTw6W81s8PdgyPcmBa72gvdetrqnYB6
> WjLycbEo80mkW0oM+15sBnaONr6B3i1sPxG7NrcchaJF7VlD/i7kinChIPJQayA=
>
>
