List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:November 25 2010 7:34am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3548)
Bug#54903
View as plain text  
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=
> 
> 


Thread
bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3548) Bug#54903Li-Bing.Song25 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3548)Bug#54903He Zhenxing25 Nov
    • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3548)Bug#54903Libing Song25 Nov