Code review comment for lp:~sergei.glushchenko/percona-server/5.5-BT34246-ps-blueprint-into-outfile-pipe-and-socket

Alexey Kopytov (akopytov) wrote :

Sergeim

   - the patch also allows using FIFOs/sockets with SELECT INTO
     DUMPFILE, rather than just SELECT INTO OUTFILE, but it’s not
     present anywhere in tests/MP/blueprint

   - please copy the MP description into the BP

   - Unix domain sockets are not supported on Windows, so all
     sockets-related code should be wrapped into #ifndef __WIN__

   - the test should also include inc/not_windows.inc

   - call to mysql_file_open() in create_file() does not need O_EXCL (it
     only makes sense together with O_CREAT which is not being passed)

   - if either mysql_file_open() or mysql_unix_socket_connect() return
     -1, create_file() assumes the file was neither a FIFO nor a socket
     and raises the ER_FILE_EXISTS_ERROR. Instead, it should not raise
     any errors (because it was already raised by one of those
     functions) and return -1.

   - if connect() fails in my_unix_socket_connect(), we return without
     closing the socket.

   - instead of doing:


  if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
    if (MyFlags & (MY_FAE | MY_WME))
      my_error(EE_CONNECT, MYF(0), FileName, errno);
    DBUG_RETURN(-1);
  }

  DBUG_RETURN(my_register_filename((File) sd, FileName, FILE_BY_OPEN,
           EE_FILENOTFOUND, MyFlags));

you could do:


  if (connect(sd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
    close(sd);
    sd= -1;
  }

  DBUG_RETURN(my_register_filename((File) sd, FileName, FILE_BY_OPEN,
           EE_CONNECT, MyFlags));

  - in the test case, the following Perl snippet creates a FIFO named
    ‘outfile’, but then the further code expects it to be named
    ‘fifo’:


perl;
use POSIX qw(mkfifo);
my $fn = "$ENV{'MYSQL_TMP_DIR'}/outfile";
unlink($fn);
mkfifo($fn, 0666) or die("mkfifo: $!");
EOF

--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
--send_eval SELECT 1,2,3 INTO OUTFILE '$MYSQL_TMP_DIR/fifo'

perl;
my $fn = "$ENV{'MYSQL_TMP_DIR'}/fifo";
open(FILE, $fn);
print(<FILE>);
close(FILE);
unlink($fn);
EOF

    The above only works because the second Perl snippet opens the file
    for reading and writing (i.e. creates a regular file named ‘fifo’)
    before SELECT INTO OUTFILE opens it. So we are testing SELECT INTO
    OUTFILE with a regular file here. Also, the second snippet is
    equivalent to:

--cat_file $MYSQL_TMP_DIR/fifo

  - the Unix domain socket test uses SLEEP() for synchronization. I
    understand it is tricky to avoid it in this case. But the following
    should work:

    1) create a procedure that would poll a certain file and execute
       SELECT INTO OUTFILE only when it is created:

CREATE PROCEDURE p1() BEGIN WHILE ISNULL(LOAD_FILE('$MYSQL_TMP_DIR/trigger')) DO SELECT SLEEP(1); END WHILE; SELECT 1,2,3 INTO OUTFILE '$MYSQL_TMP_DIR/socket'

    2) In Perl create that file before the listen() call:

open(FILE, '>', "$ENV{'MYSQL_TMP_DIR'}/trigger");
close(FILE);

review: Needs Fixing

« Back to merge proposal