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

[Sergei Golubchik]

Hi, Alfranio!

On Feb 11, Alfranio Correia wrote:
> Sergei Golubchik wrote:
> > On Feb 11, Alfranio Correia wrote:
> >>  2768 Alfranio Correia	2009-02-11
> >>       BUG#38174 secure-file-priv breaks LOAD DATA INFILE replication in
> statement mode
> >> === added file 'mysql-test/suite/rpl/t/rpl_bug38174-slave.opt'
> >> --- a/mysql-test/suite/rpl/t/rpl_bug38174-slave.opt	1970-01-01 00:00:00
> +0000
> >> +++ b/mysql-test/suite/rpl/t/rpl_bug38174-slave.opt	2009-02-11 15:39:04
> +0000
> >> @@ -0,0 +1 @@
> >> +--secure-file-priv=/tmp
> >
> > Why ?
> >   
> This is used to show that the patch works as the bug report says that
> the LOAD DATA INFILE fails in the slave when secure-file-priv is
> defined.

secure-file-priv is always defined in the mysql-test-run tests, you
don't need to specify it explicitly.

> >> === modified file 'sql/sql_load.cc'
> >> --- a/sql/sql_load.cc	2008-09-17 12:54:50 +0000
> >> +++ b/sql/sql_load.cc	2009-02-11 15:39:04 +0000
> >> @@ -310,8 +310,24 @@ int mysql_load(THD *thd,sql_exchange *ex
> >>  	is_fifo = 1;
> >>  #endif
> >>  
> >> -      if (opt_secure_file_priv &&
> >> -          strncmp(opt_secure_file_priv, name,
> strlen(opt_secure_file_priv)))
> >> +      if (thd->slave_thread)
> >> +      {
> >> +#ifdef HAVE_REPLICATION
> >
> > why #ifdef HAVE_REPLICATION ? and why here, inside if (thd->slave_thread) ?
> 
> This is related to the way things are linked.
> To resolve the reference to slave_load_tmpdir while linking the code,
> we need this #ifdef.
> 
> The problem stems from the fact that most all the includes in the
> replication
> code has an #ifdef. This seems to be used as a way to avoid linking some
> parts
> of the code while creating "client applications".
> > Can thd->slave_thread be true if HAVE_REPLICATION is not defined ? 
> >   
> I would say no based on the previous explanation.

Okay, then add

#else
  DBUG_ASSERT(FALSE); // impossible

> >> +        char buf[FN_REFLEN];
> >> +        fn_format(buf, PREFIX_SQL_LOAD, slave_load_tmpdir, "",
> >> +                  MY_PACK_FILENAME | MY_UNPACK_FILENAME |
> >> +                  MY_RETURN_REAL_PATH);
> >> +        to_unix_path(buf);
> >> +
> >> +        if (strncmp(buf, name, strlen(buf)))
> >> +        {
> > comment would be nice. E.g. "LOAD DATA INFILE in the slave thread can
> > only read from slave_load_tmpdir"

I suppose you'll add a comment...

Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring