List:Commits« Previous MessageNext Message »
From:Alexander Barkov Date:May 5 2010 10:30am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (bar:3027) Bug#51571
View as plain text  
Hi Luís,


Luís Soares wrote:
> Hi Bar,
> 
>   Nice Work. 
>   Please find my review comments below.
> 
> STATUS
> ------
>  
>   Approved (after consideration of RC1).

Thanks for review.

RC1 discussed on IRC.


> 
> REQUIRED CHANGES
> ----------------
> 
>   RC1. Seems that the test you added does not cover the change in
>        sql_load.cc. To make the test case complete I think we
>        would be better of with testing this in mixed or statement
>        mode as well. What about adding the test case to the
>        replication suite (this would cover the logging and the
>        execution on the slave)? What do you think?
> 
> REQUESTS
> --------
>  n/a 
> 
> SUGGESTIONS
> -----------
>  n/a
> 
> OBSERVATIONS
> ------------
> 
>   O1. The change in sql_load.cc makes us not setting the field if
>       it differs from Item::FIELD_ITEM, whereas before this
>       patch, we would not set the field when item->name was NULL.
> 
>       I think the change is good, however, I just want to
>       pinpoint that that we are reducing the scope to
>       FIELD_ITEM (which makes perfect sense in my mind).
> 
>       I trust that this change has no side effects.
>  
> DETAILS 
> -------
>  n/a
> 
> 
> On Tue, 2010-05-04 at 14:34 +0000, Alexander Barkov wrote:
>> #At file:///home/bar/mysql-bzr/mysql-trunk-bugfixing2/ based on
> revid:alfranio.correia@stripped
>>
>>  3027 Alexander Barkov	2010-05-04
>>       Bug#51571 load xml infile causes server crash
>>       
>>       Problem:
>>       item->name was NULL for Item_user_var_as_out_param
>>       which made strcmp(something, item->name) crash in the LOAD XML code.
>>       
>>       Fix:
>>       - item_func.h: Adding set_name() in constuctor for
> Item_user_var_as_out_param
>>       - sql_load.cc: Changing the condition in
> write_execute_load_query_log_event() which
>>       distiguished between Item_user_var_as_out_param and Item_field
>>       from
>>         if (item->name == NULL)
>>       to
>>         if (item->type() == Item::FIELD_ITEM)
>>       - loadxml.result, loadxml.test: adding tests
>>
>>     modified:
>>       mysql-test/r/loadxml.result
>>       mysql-test/t/loadxml.test
>>       sql/item_func.h
>>       sql/sql_load.cc
>> === modified file 'mysql-test/r/loadxml.result'
>> --- a/mysql-test/r/loadxml.result	2009-11-11 17:30:51 +0000
>> +++ b/mysql-test/r/loadxml.result	2010-05-04 14:27:32 +0000
>> @@ -73,3 +73,23 @@ id	text
>>  line2
>>  line3
>>  drop table t1;
>> +#
>> +# Bug#51571 load xml infile causes server crash
>> +#
>> +CREATE TABLE t1 (a text, b text);
>> +LOAD XML INFILE '../../std_data/loadxml.dat' INTO TABLE t1
>> +ROWS IDENTIFIED BY '<row>' (a,@b) SET b=concat('!',@b);
>> +SELECT * FROM t1 ORDER BY a;
>> +a	b
>> +1	!b1
>> +11	!b11
>> +111	!b111
>> +112	!b112 & < > " ' &unknown; -- check entities
>> +2	!b2
>> +212	!b212
>> +213	!b213
>> +214	!b214
>> +215	!b215
>> +216	!&bb b;
>> +3	!b3
>> +DROP TABLE t1;
>>
>> === modified file 'mysql-test/t/loadxml.test'
>> --- a/mysql-test/t/loadxml.test	2010-01-05 21:36:08 +0000
>> +++ b/mysql-test/t/loadxml.test	2010-05-04 14:27:32 +0000
>> @@ -108,3 +108,11 @@ load xml infile '../../std_data/loadxml2
>>  select * from t1;
>>  drop table t1;
>>  
>> +--echo #
>> +--echo # Bug#51571 load xml infile causes server crash
>> +--echo #
>> +CREATE TABLE t1 (a text, b text);
>> +LOAD XML INFILE '../../std_data/loadxml.dat' INTO TABLE t1
>> +ROWS IDENTIFIED BY '<row>' (a,@b) SET b=concat('!',@b);
>> +SELECT * FROM t1 ORDER BY a;
>> +DROP TABLE t1;
>>
>> === modified file 'sql/item_func.h'
>> --- a/sql/item_func.h	2010-03-31 14:05:33 +0000
>> +++ b/sql/item_func.h	2010-05-04 14:27:32 +0000
>> @@ -1498,7 +1498,8 @@ class Item_user_var_as_out_param :public
>>    LEX_STRING name;
>>    user_var_entry *entry;
>>  public:
>> -  Item_user_var_as_out_param(LEX_STRING a) : name(a) {}
>> +  Item_user_var_as_out_param(LEX_STRING a) : name(a)
>> +  { set_name(a.str, 0, system_charset_info); }
>>    /* We should return something different from FIELD_ITEM here */
>>    enum Type type() const { return STRING_ITEM;}
>>    double val_real();
>>
>> === modified file 'sql/sql_load.cc'
>> --- a/sql/sql_load.cc	2010-03-31 14:05:33 +0000
>> +++ b/sql/sql_load.cc	2010-05-04 14:27:32 +0000
>> @@ -696,7 +696,7 @@ static bool write_execute_load_query_log
>>      {
>>        if (n++)
>>          pfields.append(", ");
>> -      if (item->name)
>> +      if (item->type() == Item::FIELD_ITEM)
>>        {
>>          pfields.append("`");
>>          pfields.append(item->name);
>>
>> text/bzr-bundle type attachment
>> (bzr/bar@stripped)
>> # Bazaar merge directive format 2 (Bazaar 0.90)
>> # revision_id: bar@stripped
>> # target_branch: file:///home/bar/mysql-bzr/mysql-trunk-bugfixing2/
>> # testament_sha1: c840ac4e9e3818a9813650e3e068ced690268bed
>> # timestamp: 2010-05-04 18:34:55 +0400
>> # base_revision_id: alfranio.correia@stripped\
>> #   173s6pgmn913gscc
>> # 
>> # Begin bundle
>> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWYX5mbIABCHfgHAweff//3+n
>> 3uC////6YAj317tZxQAAVJbaApWgDQASlJlPMin6kyfqnppHqA09Q9TEyZpH6oNDTQAA0GijVP0E
>> nmlPKeU/VPU0w0R6gaZDJiM1G0jTEGJjgGEYTTEMAgGQAwjTJkwjAQ0EiQk00ak9E02T1Rsp6NE0
>> ek0A09TRoBkaNqAOAYRhNMQwCAZADCNMmTCMBDQSSEyAEwhPQTIAQyCmyjGieo00PUGaiGJpbBRl
>> JyLYwnWHLQy5zuTD/OUxooeUFBtg/lLcL8ttCX+Cb50r61GFluU2pSQy3M3dF1zqyemUXjTjk4QW
>> W6rT3ZUyb7SXjNEM8EUpNkIIy4f21NeHsJAgHSWULaIUgRgbbY2np+/SLsnY7ojQyLqh7VKVNubb
>> Ipk5GaMW04zHzwkGMznaTjilRNRXvQ2yyd5cM41O6gbx7ph4CKAscnGycZcWpA2mEJROFh1vwd+y
>> Sp5ZGwmFSQHzAHaZOnvojbJOHmPEMx0kYGrBWFNg1klw9lweEH3X8DsauWVu8R5zLe8r1Q6VhiIc
>> RpERbwSQL17Ix40/fx5gktRsnwKiYSNtO9IkxsNefM7vFKwzlcFhI57DjgXtlQcjDFagocunvZtq
>> 4nEyLEUJB6jeO3+MbV/EqocTHI/kC6+ouYmLdu/WfQ/INKqAnXghgXBMbAYN7BOb0R87AiZn3oYk
>> WWPdjSkHB7ht8zo/MLwIhFGIJhIGwyVUNwZSgNQCahVWjwW2EiYLSBkWUlZChDBOiqncPHFHjUO5
>> /Mm6mOkx63jkcgZXOL1EujN+jGtkgYGBk1momratq2e+TnYKsmzGpD0sI1rY8eSLSxnNMNL4WqYI
>> VGSJ1rBQniWTKorUNRgnE3RqOq2IaD9ZejyCJYPO8YgjEzMi77G20q6GzoxjC0odF4LONAC1xCmh
>> mi52Q5zdBuqjtIGJ9jDG+sZ51mq54Kk3Tj1jRFKy64Hn7MbrSR9gTEMZWvyJKM5fIIEG1ES4nJgT
>> UYkLigdmxAzIj1KppwiDgoHmBEtJisZczsNpnUZgrCxVmg5wU1DOrsfozZRFesnBNHock4gCvHpJ
>> ysDAcsSBWWjHaWHbz3lhSxmMPSnIGAPsLSkx+ymLu9oT31Q3RaqEiZb5FgLBuyeJ9FPNTJwuRqWM
>> CddJeE+Rv9QLqLn8YGhTZeVXjy4rgRMZGRmcJTnbzJSVxkL1mRuK3WHcnnUakXFKcYFYxZ0nE/X9
>> YAomtcMR+0aBttHTCrNgKgcTmWVML6aVLA4ctDMuLabCghTtJyBImPAF0lx5Eaar2axFkwVJvBVl
>> K0MDdEJQNlAVkZmKzUTGBWWnP06rNOi+lom0daMd7nXDkaxPejUlM6zb2tQPHGwLY0qtoPUzzHMs
>> uHdV8ZHkd1oMibY5mAatXHL3o8vB8XFEuwfCb0+8LLa3ApGtfiL6L8S3Bh3BtjY2+sU0UyQTJEMm
>> uw6ih7QwDFIlBMOkofUIJzOE1mAriDWF8ZBYz+jS+SsLSo9onH0/YsJ8yY+ynThiBaUqwHjEieQ4
>> /clcYkT7DFBqkUDxZlhiGbFhMo4hWatwe3tPy3PRoUGcGm0ucHxSN8GFbql6ZFGlWFgY8q2WmkVF
>> QsQxFcyHEUjxARFRYyDTqOgLB4osUSSaPP2fI+678wK9MniKiqJol866y+AcEjR1n4aD1VWhYAxA
>> HBVzcgVh27jeIIGe6S4C/124j/B1heYuqDpOwF9C0gTr+2sq5H1/M6RMH72zkOVPedfCbaC/neXH
>> rSHHax2B3GI7wHnkbSgl4kjUqaDyPqP+oMcSg2bz1lTY99mARmO+FYIb9BhyYKY1gp++txBbzz/Q
>> HhAsu0kS6DYPKyoXQSFSP2FgVgvQtoMDMiffMctpuK4ckdxfDpn1EhGXr89EzBS+FQ4HwDWWvJy1
>> Vl+2L3+J8PQkUmhjeFyJHRuMmTsEQSdQZVFpwT/c2GZgeJnl1/5GXNxzvRgwLL3yDscnAWwnkq4g
>> 4XmBqMEJm73SoTdgUCXHo8JRIxSvIUaFfuRmcR5WeXqibExxOveNQkxwK0dmw1da/0HeHFGnYrjh
>> I+6KNFPlUGMlxTb8nc14QxQQi3VkpyjwLlRjgZUb+VXN6U8ILiJSmIfQ+E7kTqJ3yapaPy1oIrrW
>> fiNnqdtBZyWh/7xQ+s4xAHkQgkyYqI3mXnbdCl2UtseEJ5UQY2Ra6xTMPxxXc0sIwKOR8F7Dw4Lp
>> WWBWUG/keztOKU/bRbYXtoWLUIGEQFk8WrqAMfTiFC6Fej4G57nDM5+ZYRDwsNu/Td5djXJmxByQ
>> MJW7V3kyRh165221HYhQrXEtpqRWQ6P1+SS4luFbmY2IdcXroeg+Ju1s5yJB6TGBSkzhwhxWymPK
>> vyNZkUXoKRgGUpaTp0FmYG4QU7WdJzwRxaQJl3NRMiI4pAZQXm9WnTLYFzjuutQe0E7bIGDnz5Cn
>> ij2lgWHvV+kSmkZ+ZSFOGHWa8RIuGN1pcY10m6cZNvfNBPQDBBJwdei0CUtFEF7g1+e4NrJ+NpR8
>> ezkValodJ++rGnYD7SdX7SGb4KkLCTIblSFQuGomNkwebFUWcveXfDq1pyq0eRKwijUjrBMHncLr
>> Pb60cGZu8gAOcVvMy4+IyUiFdiXOePFg9SJthNhue4Jd4QnmHGUgPf3AptIsFJxqClbglWA0Rk6+
>> DI+hBnAwdGAVlyEzFPiWT8CmI+2bv8IpQTuQ8eb3OMClwqIVwPPz4jwWRb7D3Zxf+C0wBaYk543o
>> GCz3Py4gyuCmkdMe06ZBnsVS3nyge5kifqieJ6j17bjnMboD+ehkSh/A4GHV1YDRXAREgharWx3D
>> 3lLiz4GIg3PlYRKByPGairA6d2o/+LuSKcKEhC/MzZA=
>>
>>
> 
> 

Thread
bzr commit into mysql-trunk-bugfixing branch (bar:3027) Bug#51571Alexander Barkov4 May
  • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3027) Bug#51571Luís Soares5 May
    • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3027) Bug#51571Alexander Barkov5 May