Hi Alexey,

> 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
> 

I've mentioned it in both comments/MP and modified the test case to verify this behaviour. I think it's a feature rather than bug.

>    - please copy the MP description into the BP
> 
done.

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

done.

>    - the test should also include inc/not_windows.inc
> 

done.

>    - 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)
> 

right, it doesn't make sense. removed.

>    - 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.
> 

We need to raise ER_FILE_EXISTS_ERROR, in case when file exists and is not socket/fifo, but you're right that we did it also in the case of other errors. Fixed.

>    - 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));
> —
> 

Done.


>   - 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.

Fixed.

> Also, the second snippet is
>     equivalent to:
> 
> --cat_file $MYSQL_TMP_DIR/fifo
> 

Replaced perl with cat_file.

>   - 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);

The method you suggested is not reliable either. If mysqld catch the moment between file created and listen() entered it will not be able to connect with "Connection refused", but it looks better than sleep(2) anyway, so I implemented it.