Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2769) Bug#38174

[Alfranio Correia]

Hi Sergei,

Thank you for the comments.
I will changed what you requested before pushing.

Sergei Golubchik wrote:
> Hi, Alfranio!
>
> Thanks
> ok to push
>
> A couple of comments, see below (but ok to push anyway)
>
> On Feb 12, Alfranio Correia wrote:
>   
>> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-38174/mysql-5.1-bugteam/
> based on revid:alfranio.correia@stripped
>>
>>  2769 Alfranio Correia	2009-02-12
>>       BUG#38174 secure-file-priv breaks LOAD DATA INFILE replication in statement
> mode
>> removed:
>>   mysql-test/suite/rpl/t/rpl_bug38174-slave.opt
>>     
>
> it would've been better to uncommit the previous changeset and remove
> the file - then from the bzr point of view it would've never existed.
> Here you committed it and removed it later, it leaves the trace in the
> history, absolutely unnecessary (and one could restore the deleted file,
> so its content is also preserved in the history).
>
>   
I was planning to do that. Right now, I only wanted to commit/highlight
the differences between the two patches.
>> modified:
>>   sql/sql_load.cc
>>
>> === removed file 'mysql-test/suite/rpl/t/rpl_bug38174-slave.opt'
>> --- a/mysql-test/suite/rpl/t/rpl_bug38174-slave.opt	2009-02-11 15:39:04 +0000
>> +++ b/mysql-test/suite/rpl/t/rpl_bug38174-slave.opt	1970-01-01 00:00:00 +0000
>> @@ -1 +0,0 @@
>> ---secure-file-priv=/tmp
>>
>> === modified file 'sql/sql_load.cc'
>> --- a/sql/sql_load.cc	2009-02-11 15:39:04 +0000
>> +++ b/sql/sql_load.cc	2009-02-12 06:51:01 +0000
>> @@ -321,9 +321,15 @@ int mysql_load(THD *thd,sql_exchange *ex
>>  
>>          if (strncmp(buf, name, strlen(buf)))
>>          {
>> +          /*
>> +            LOAD DATA INFILE in the SQL Thread can only read from 
>>     
>
> I'd prefer "slave SQL thread". Simply "SQL thread" is confusing, can
> mean a connection thread for those who don't live and breathe
> replication - and this file, sql_load.cc, isn't replication file, "SQL
> thread" in slave.cc is less ambiguous.
>   
Ok !!! I will change it.
> but ok, it's inside thd->slave_thread, hopefully, it's sufficient
>
>   
>> +            --slave-load-tmpdir"
>> +           */ 
>>            my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
> "--slave-load-tmpdir");
>>            DBUG_RETURN(TRUE);
>>          }
>> +#else
>> +        DBUG_ASSERT(FALSE); // This is impossible
>>  #endif
>>        }
>>        else if (opt_secure_file_priv &&
>>
>>     
> Regards / Mit vielen Grüßen,
> Sergei
>
>   
Cheers.