List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:March 6 2009 2:31pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:2801)
Bug#39701
View as plain text  
Luis, hello.

The patch is fine, still I would like to improve the test part.
There are 2 suggestions.
Pls let me know about their resolution, and I'll approve technically
at instant.

cheers,

Andrei

>
> === modified file 'mysql-test/suite/rpl/t/rpl_loadfile.test'
> --- a/mysql-test/suite/rpl/t/rpl_loadfile.test	2007-12-12 17:19:24 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_loadfile.test	2009-03-03 23:30:51 +0000
> @@ -11,7 +11,7 @@
>  
>  # Includes
>  -- source include/master-slave.inc
> -
> +-- source include/have_binlog_format_mixed_or_row.inc
>  
>  # Begin clean up test section
>  --disable_warnings
> @@ -51,3 +51,99 @@ DROP TABLE test.t1;
>  sync_slave_with_master;
>  
>  # End of 5.0 test case
> +
> +#  BUG#39701: Mixed binlog format does not switch to row mode on LOAD_FILE
> +#
> +#  DESCRIPTION
> +#
> +#    Problem: when using load_file string function and mixed binlogging format
> +#             there was no switch to row based binlogging format. This leads
> +#             to scenarios on which the slave replicates the statement and it
> +#             will try to load the file from local file system, which in most
> +#             likely it will not exist.
> +#
> +#    Solution:
> +#             Marking this function as unsafe for statement format, makes the
> +#             statement using it to be logged in row based format. As such, data 
> +#             replicated from the master, becomes the content of the loaded file.
> +#             Consequently, the slave receives the necessary data to complete
> +#             the load_file instruction correctly.
> +#
> +#  IMPLEMENTATION
> +#
> +#    The test is implemented as follows:
> +#
> +#      On Master,
> +#       i) write to file the desired content.
> +#      ii) create table and stored procedure with load_file
> +#     iii) stop slave
> +#     iii) execute load_file 
> +#      iv) remove file
> +#
> +#      On Slave,
> +#       v) start slave
> +#      vi) sync it with master so that it gets the updates from binlog (which 
> +#          should have bin logged in row format). 
> +#
> +#          If the the binlog format does not change to row, then the assertion
> +#          done in the following step fails. This happens because tables differ 
> +#          since the file does not exist anymore, meaning that when slave 
> +#          attempts to execute LOAD_FILE statement it inserts NULL on table 
> +#          instead of the same contents that the master loaded when it executed 
> +#          the procedure (which was executed when file existed).
> +#
> +#     vii) assert that the contents of master and slave 
> +#          table are the same
> +
> +connection master;
> +source include/reset_master_and_slave.inc;
> +
> +connection master;
> +
> +SELECT repeat('x',20) INTO OUTFILE '../../tmp/bug_39701.data';

I think $MYSQLTEST_VARDIR/tmp is better and safer than the relative path.

> +
> +disable_warnings;
> +DROP TABLE IF EXISTS t1;
> +enable_warnings;
> +
> +CREATE TABLE t1 (t text);
> +DELIMITER |;
> +CREATE PROCEDURE p() 
> +  BEGIN
> +    INSERT INTO t1 VALUES (LOAD_FILE('../../tmp/bug_39701.data'));
> +  END|
> +DELIMITER ;|
> +
> +# stop slave before issuing the load_file on master
> +connection slave;
> +source include/stop_slave.inc;
> +
> +connection master;
> +
> +# test: check that logging falls back to rbr.
> +CALL p();
> +
> +# test: remove the file from the filesystem and assert that slave still 
> +#       gets the loaded file
> +remove_file $MYSQLTEST_VARDIR/tmp/bug_39701.data;
> +
> +# now that the file is removed it is safe (regarding what we want to test) 
> +# to start slave
> +connection slave;
> +source include/start_slave.inc;
> +
> +connection master;
> +sync_slave_with_master;
> +
> +# assertion: assert that the slave got the updates even
> +#            if the file was removed before the slave started,
> +#            meaning that contents were indeed transfered
> +#            through binlog (in row format)
> +let $diff_table_1=master:test.t1;
> +let $diff_table_2=slave:test.t1;
> +source include/diff_tables.inc;
> +
> +# CLEAN UP
> +DROP TABLE t1;
> +DROP PROCEDURE p;
> +sync_slave_with_master;
>

So you have renamed the former rpl_loadfile test to _stm_ as we discussed.
As the former test content exists in the new rpl_loadfile I think it
is necessary to `source' the common tests into either test files
instead of the current copy-paste.

> === added file 'mysql-test/suite/rpl/t/rpl_stm_loadfile.test'
> --- a/mysql-test/suite/rpl/t/rpl_stm_loadfile.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_stm_loadfile.test	2009-03-03 23:30:51 +0000
> @@ -0,0 +1,57 @@
> +#############################################################################
> +# Original Author: JBM                                                      #
> +# Original Date: Aug/18/2005                                                #
> +#############################################################################
> +# TEST: To test the LOAD_FILE() in rbr                                      #
> +#############################################################################
> +# Change Author: JBM
> +# Change Date: 2006-01-16
> +# Change: Added Order by for NDB
> +# Change: Split the original test file. This one forces STATEMENT only because
> +#         when in STATEMENT mode, the load_file will issue a warning, whereas
> +#         in RBR or MIXED mode it does not (by lsoares).
> +##########
> +
> +# Includes
> +-- source include/master-slave.inc
> +-- source include/have_binlog_format_statement.inc
> +
> +
> +# Begin clean up test section
> +--disable_warnings
> +connection master;
> +DROP PROCEDURE IF EXISTS test.p1;
> +DROP TABLE IF EXISTS test.t1;
> +--enable_warnings
> +
> +# Section 1 test 
> +
> +CREATE TABLE test.t1 (a INT, blob_column LONGBLOB, PRIMARY KEY(a));
> +INSERT INTO test.t1  VALUES(1,'test');
> +UPDATE test.t1 SET blob_column=LOAD_FILE('../../std_data/words2.dat') WHERE a=1;
> +delimiter |;
> +create procedure test.p1()
> +begin
> +  INSERT INTO test.t1  VALUES(2,'test');
> +  UPDATE test.t1 SET blob_column=LOAD_FILE('../../std_data/words2.dat') WHERE a=2;
> +end|
> +delimiter ;|
> +
> +CALL test.p1();
> +SELECT * FROM test.t1 ORDER BY blob_column;
> +save_master_pos;
> +sync_slave_with_master;
> +connection slave;
> +# Need to allow some time when NDB engine is used for
> +# the injector thread to have time to populate binlog
> +let $wait_condition= SELECT INSTR(blob_column,'aberration') > 0 FROM test.t1
> WHERE a = 2;
> +--source include/wait_condition.inc
> +SELECT * FROM test.t1 ORDER BY blob_column;
> +
> +# Cleanup
> +connection master;
> +DROP PROCEDURE IF EXISTS test.p1;
> +DROP TABLE test.t1;
> +sync_slave_with_master;
> +
> +# End of 5.0 test case
>
> === modified file 'sql/item_create.cc'
> --- a/sql/item_create.cc	2008-04-11 03:27:24 +0000
> +++ b/sql/item_create.cc	2009-03-03 23:30:51 +0000
> @@ -3791,6 +3791,7 @@ Create_func_load_file Create_func_load_f
>  Item*
>  Create_func_load_file::create(THD *thd, Item *arg1)
>  {
> +  thd->lex->set_stmt_unsafe();
>    thd->lex->uncacheable(UNCACHEABLE_SIDEEFFECT);
>    return new (thd->mem_root) Item_load_file(arg1);
>  }
>
>
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: luis.soares@stripped
> # target_branch: file:///home/lsoares/Workspace/mysql-\
> #   server/bugfix/39701/5.1-bt/
> # testament_sha1: 22b1d2cbb5b027814768698631dbaafeecb9d631
> # timestamp: 2009-03-04 00:31:00 +0100
> # base_revision_id: alexey.kopytov@stripped\
> #   8gqbau6h7mccfozk
> # 
> # Begin bundle
> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWTLhsxkACMdfgHtwXff//3//
> /+C////0YBZeu75Lvvd4OtKO93rKge+xq62zQe095zQuW6ejN53VznM2CrrWxtrtu7Y55PW97iiV
> b3cJNG9vYSSmRpoJoaNTTJiZMqe1T9TyofqTyj1PRMjI0AyA9TygShNAmgFNpomp6qeMk1P0UeiZ
> pvVI0aANGgAA0Gp6mSmamhMQeoaaZqZD1DQA0aGCAAAaGmEhQgRTYjVH5KPam0p6Q9NTR6Ahpppi
> Mm0gAABFJE9U9TFPU9op6ein6oaAep6gbUyYgAZDygaADQSIiBMQBGImEYiek1PImhppo0aGho0N
> AaKyxsvoGCFPcQZi4QesoXi28XXa/vI5YOQ7MjVFLUS8mdr2z7uFjBMzFLfv4/5T5FxNxOPqwvg0
> z8DN8tcLX9CUFjVQ2hGjliLAXecD6MyENedGaCOe/V5FZTdMRpYPy2Edl6bdMuF4pYr0SYihBfAJ
> gmwf4CgJSZ8Pex3a65079KhuZlS0tP1wan6GZGu3nTAnRMS4XUMK7WG0OoJA/sEhAYAVCQqFA4gw
> 8QqHgEAiBAJBAJhiEgUIByCYSBwcKgoUCYSDcxQSP+mMZ0bRwOMKwZD/2PqPb5HUnr9PdXCGRgY+
> 27z3V6Ty1mrcd9he+LRTvgP92cmJBGrJPSxoyyZ9EDRTRSIQc20EFGHdKKWUZmMIM6vBY9s0fcYF
> tk1YQpD7weXyi+Bc6bbYNnR7v8AZ7RLImbZaWwJFK1LL4Vvl5JtESppfaz4h6OkkyrBjsZYhA7kr
> JrWP3gV+YM7V3AWHXkZECuyPvORmMr2a5sGbxenQP5HhA9p8waCle3x+i4MomFzNlULhRRSOyD+/
> jcKC7BGl2vAgC+zT3ev8/KqpfQjqxHe9mxVdI6z9558ivyd+E2reprrWDiqRPMHMKd2s4WSa6Ylp
> WUIvrPKNaTIKdnebxdgb4j4YQv9DGE2E4WYk9xAwURnd3Sv+1TiRVSUPkKTFV2eDRYZ66ua++U5x
> xJt4oXZiyuV7AbTeeHJi4OpB6sJizCxTHVPwTM0tVCao+/eSHta0MtKvI3tO9SUQqpi9WgfYz3n3
> 87nMF1uRvROgJN0JsnrxkuitSjn+so0U86snIkKb3oU4ySdy5ZRjx76GJGIaWQ3UlGqYMGAz6JUi
> M3VieRBxHWA8HVieX1pWL53XUV2GNGTjNm/Kb8PRoki2pgewtPmkeNdQ8vTHmzayrzGcS8LwsfQC
> gqwF1NDiIZNlbxgQ3jq7nlD0K8c+LxiP3ryF8HHamVilZTlN5OeFZ3UjWRJaTjWNHeMpjLkXNLpU
> 0KXF1+PTCkufUgnBbgWWtvNWK53E0wiM0ygnsM0hXUnqXo0OSsQ8UoA5I2hDAcymteHercvHxRy9
> kL/ps5xg39Ha1JGLkO0+gsf3dXYknW4cj0ha+9BHZokElQYMX8xrQHi+vj0B4DDkfYKwoaIYSd4r
> jkPjKPKhOkMA8BzxFw0eTIybfi4xhzBR/6M826Gs+HxUv+K8dskKZTPoMgYPn56sH+Uc/MYe/ZXM
> bWbz0aXqjlqsl26RsRISHmOJxOBwOk7VzifMvC8zUtJ8yuScMy0CYS7MRsXN5qA5S2HK9llUpOGh
> wnW0sXBYh8q1GIlC6p7eyM51TkyXtrtZE0k6gP5Ad4BnL3NMI04BkBZpCZFWqATNeZQUprYC+qJV
> SUpIlSlkBt7e8XuRVaccZF9qKEQw4Y1IiFK0A1IWEh8dPMN3Vkg8Wgt014HQqz4Olq6YEBHa0UVF
> 2EnYkiViWVBE5luPgUmqSYsBdxGOB4GJJdUkoRiXmJKSLyVeldlQxLz21XkaDUBw0MUMYwTxocw8
> ypTct7SUI2MWi+umVratX48thvKC31qTb5VsdeGBJivMY+InFqRJz+w2jlgMikCmfrrbUj7fRbgc
> J2TUqTwOInIOxIMg/uouJlIRzFpRjjHaXTOG+V2QM4Ecw2F+5dRJQTFiu4aulwARRcdR19BRHLlf
> oCmGgrZUsTNBkY4SBXidOZxOJBcdxccFioOS6RKzxM77HhJttIUHAEMS36yVGEFHu3wCLB4d5cIj
> akTB5bIsi8R40LF6eneTRJkhUskUoMcUOxx26S7IKEayppM/Fc6CVa6XnMxqjea1tlVyLiBPTGOQ
> 8CcckzJkHZOH4mR8CO8rr7i70+IQMhOZI4GIqPHgNYo3GKEZLX2PwFxm9vsUyiksaYWSoceYnFw+
> 0xSgmxxT7a8UqeJ3P5yg56ktjebaamGoupBMUxKdhmdCtJNUh7VK2qq81hhWkcTVMpjxNhYUK8Lx
> yEtxrmIEDJiLroQxkCtiMUrnjyQyiVhYWXzMsRTZIpunkVOhR8FythjxNN0Q1c3JwY+HWaWDY2OX
> LdNImBDGhiZwIilksZIZ5+z4M6J0oMcTXXeWBsOpYHkuRzSVmTeke/ambqlKdY96lcKls8B+EBzk
> UJGp8SVa4vVhqCTREIBWAp4jjkypeTfgU6mWMLk9LJiBcneYnPneS4EyZkMKVOBmZp4eZTqnmJjd
> o31X5+E1Qxw5ZZ8zRDeLIKJFaRdEVEZRGeC1JMAuWYvHOBwXt1rDZXmnZisrfW8BZxWBKjBFPqVB
> ryVIUFqS7Q2nSDTHhDC26dT3fGVeCO41bNbBhk7yIy5Kq5EIMY0GsKBbTv8Avfqm+JduVZrHgVKf
> QmJZd4L9Yl94f9Fzf1qBfuC4PPtBBp6SQLGRqqmJtqr/cF6T+Tf2bkdh3hiBIL1VSoij9FD2B7Cx
> iFzglYDOJVLGEZhBWMkkwELIIExxQNUl/xkA3jAvOIqame4Hygx4IgLyE+olHinA8RvgB95KMCcK
> cwFo73ob3JgPgAVGAd0H22pzMkVk2C4DcH9rEYC3bQ4AwYDA6NiXJm/kD1YDG/4h9pOAz8LReyh2
> oyLNAQQP/ndKmo7pLChRKEyIAobAMQBYjSSJC0jH5JNdQsBtUCbHHhoRpeNYsvDLAKIYcBbzgQwO
> aOQlfaK4jxkxAaLwozsFJKPviFbTMClFrvjgZo3isFcpGbbATUYIWBNOGw2WWgWCLMmGl3C3FhNE
> 9gG41yLu0UlFgPM1AazQEHDyP7oyEuj2UONHfONOAaYWokmlguJAOtnCQQkQOoK1d+Ni/vX1vC9H
> haPG23jPMDsLIIJnrUKAbDQFNnSMr+NaUcCow4AVx9J6TrPwIeg8xU9ZrNZxF/AGovc90hiAAsIA
> B1VL0ZxEnHZbKX8x9RCDM7ICjSQzgXoDsA62vtLTBZBqY6JFWd3E6ZrKqydLBME1+ydI49Inp6ME
> nuU2FTatr1hEtqAektPX9RikxmtLZnhRsa18AYxIIFeBe0Bm9I20wUdIXlAJYSkZ3BuDx0XAewiE
> QecAyAFBHCA5RDQIhENgUGVgPpopRoSzaTYIZsgMz23LwOR5H2mg8vA2rmhUDuI5XriBAwSX3Jny
> AwWQwIDyIAUBgvYSAi3QFQgCQZ5EgiUFALVVBsGHpFTLcOODAO0TgGc3SALr4EPyQ8JItrAvpDag
> yGUibTMZiGwqWcHASGMrRoREG/eh/PwTqRNwLrBK4NSXqJtBL/OodGQalzY81lR1pYSltyFpBF9x
> Q2oIlV8RQlUZAXMHipEDBsGwGjQmJKjSO0wjabzwIiCAbIaXh7kQM5miTkbDiHI6EbjIeKTLgdFL
> MS3h8wlcDkEJYmMMQwYgbSrBjWRMaG0HAF7100TxcQcg2pzZlCSPqO7EVK2dV15X3IZGoFvrmK4N
> EPiWZaxLDQCWQP+UTq2aP1ZnbdLRgEAYFBQeOkmgqr95c4bFJV3WdVHmSYqoWWNllBAYRmGjewQd
> icYw6gYE4OqwtakiChkNX0ksOIsZK+iRGnVVulQdiTG6p+kz4ZyPgzZF3dmObCRjwuV5Vla0hTBy
> rIsUXAUEpTQyQaW09pjct5mXemyHZuVvkNYJ6G3iBw6Q4PYlUprVrWRcmkVAigSIgNkvbRXJz6BQ
> NDcO4hjR9/qdAj3f1QaQSCYYIJp3NtpVJ1kMSwXUnOSzrAVmaVIMnfwOZAKWQ8ln+RULY+00KM71
> 8AuGF2QbWjQDBEgQMEPVqDjDWIl0Z6u/I5kHquCbiEIuY356ER0lVAFURRBDNbRmOUdLRRpVceiB
> LWzRtDv2K+/RzpYhSFo0w0QigXgY7MZH6UcJ1ZRfdCat2EJLmmaX+d54JZTeJJSysgXowXY1F5FO
> HRvkEFhRDTSvQtwZphcjHZqHxcMf0R4/iEP1FjO/wBi6ipuWk941rERUXjHp0CLLlw7rYw3fsiBL
> bGnyoQSP5WEiVEfOiRSJnt8O08qAbRpLI51Eu/lFvL8Eg5Gxp9EImMYA4JmUhlT11c4ZEzhhGxSe
> FVBzsqKU5E0FxVE5oUoHlzds37w2e087ImmwGjWmEqC9JGPPVC5nD5SAF+loAbDlZcFofS0VYxuQ
> 8VIoCUXg24upQjOpMNy7QKcYcD4t1e4qX/IEAWKkRSJ/PHqSCWJSeTdYicUlP4xOvxKNSSQWmOtk
> vf8Z66BaZdoz8GR0Qj6WkXhRH0gsaI5jY1yAZWfbyMg6eYXhIMpxuTkptmoaPOsX0EY6gT+h6/hq
> m8QZMSbTEMDE95ypnY9YoGNDG2NkSXAbFuBVXiSYuNsYOprRnwA7+xdGUmQVEHRkOn3y8vYQDcWo
> jcvWs/qXSu1cy4rM2Gi+ZKlNPrpepXxAVPeqrxXwMFRcAqjEY20201+sDeFyLjGOSFpegPWwIknP
> 6w2hvYYYSYYBCQwyAKHib/PC2TJI8ke6lk0B0JfpgQQdwG161GY9OKvEftYckkZoUG1JjTTMCwG6
> oJxdzFwTSWiXluN6TIviT/qVZTeyrknEAGYkOgmJhoVUxbWGT/PiI2m0zmQEOhbC3EZE7XSiBggA
> v0oX/NU7ogx66Jj25pLmgm8gV4bhbJfRci2OEVkkJvSvOTnWZYhvJQiEKJItiKggUCsRAYQZKnXK
> qVC1IVSkAaUigm4NOjKK5l3CIIZisKgxMGlYvQaEoaAoBp8UCA8zBeh51RHWxUAeEMiJqHh2oNdE
> JFgrIA15Kg2xE+4YK9IvSXH4WUC8GDyFBbfWpIwJBiTaRRLSoRRqlMnaJOgWLuURw+eXfhp4bOkK
> FenYAKEayCA3WYst1Mg3ECbgbz1kbtSy3GFCeyBDB/I9wqBohAxoY0DTEoGEk1eTJQUAsISu1JjV
> YFt81lUHVtkYnjAE7QexqQkjugFw4VVLxp+AcQTbY73Azy4AWSqDoL7nB/iTEzXVzyRGwMEdgGQg
> 61qNxYWXKNxCaNGdZ0yTH88BlJjhJRF5dPKWKsyaYumQd0SvUZ/Qe3ZZmq+uIXXJh9tTLHl1LACl
> 7xLasaaxRcDBm6RjFqhBjRrPUpbQQ6RXEQDQcKt6uQFXQ0AWUD34yFIpJJMSTMnw8q94F188rXq5
> WF7MBJr52SSeCaSg2U86pQbAF12Sixqcs08du01Kk5LxNxWU0BXrYSUaRxry+5hf60sdZUXHqEV4
> Qi+buqkihOmSWJRuGlB2N6YaVIMNcIlhp9glRfGWWVGemdd1Q3gwhgwGDD1AwkDiFtvbW1xT22yU
> LIHFhKsaJo1CZQvwKBVtnlVWJsbdlatz3lUowiiSsDSvCt2cM9+IwBtYG3gTTXElB74aNbC5wbpE
> R0YFxtvFK1CmAX+Sd81LC8MRbRQF0rz4XRQMgyBVtRpRkAaeuiU/OxQFSOjySXBTIazGiax5Rcr4
> pJJAsq9kVGHlWgDhE9ZwrjpGJjEmxCLzHFQC9k7aBdbaaAuwEJ2gmTzqoBxGJIWKUIBQg5CCfIQR
> zpqQ4LCpx59JlguOQGocg9wQB1jJapNndCEIqisG7kUrCQkI7N3QLlbtFnalVzlQpZF4y0vKEIZF
> xUgGDTfSO+ioo3kW1Q7iUkuAw5pahgMYxjbTX4G8SYi5hUu78fdYaSqM50iNSiS8OklIzO5mwY6i
> XyjA6VgrcUUDCQO9jao0MGVgKyB0BXOYHTPVA2FqKALaXpVoDqxfE4wCzKgvbWlIZ9QSJgdlpJtM
> ekGZgqQFN5kjBDCoCOZgb2uDEiXqaWsf6Lg8eyw12BUDYMKb4BiBrLCkSFA3rmMi8R2IZgAs2iBU
> Q0DCq1o48FYwCIOBZdVJC8z3MEVRJhhiSVeEgz5KxYdrYKrAMRKT4jb5tGJlWv3WpKQNVfRKCiEx
> BC7dw3r2wq/wVwGkO85f37VvC9tcUbzV6t/2ZK4BtJtJDaGc6ajWpJLvX+7KoC+xgvmMhLcJa+pb
> uUS5/yzBuN0g0ZZL7FQOtgdTbHC552JQlQbd8KLpBObwPgNwyErSSWjayE4VaoBVTsUCXUSHsCwL
> TImGaT0E0NBclK9cVK9EVOOyrqVV9I1xDQIcGwqUrhwlKYi/uHNLWPcq3qnG7Gh2faS5Ii0/4u5I
> pwoSBlw2YyA=
Thread
bzr commit into mysql-5.1-bugteam branch (luis.soares:2801) Bug#39701Luis Soares4 Mar
  • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:2801)Bug#39701Andrei Elkin6 Mar