Sergei, hallo.
> Hi, Andrei!
>
> Looks better!
There's never a limit to improvements though :-).
Thanks for checking and insightful comments as usual!
I am publishing an updated patch to cover all of them.
To your notes, I am replying and commenting on a few of your questions below.
Could you also please clarify on one point, which is
>> -my_b_copy_to_file(IO_CACHE *cache, FILE *file)
>> +my_b_copy_to_file(IO_CACHE *cache, FILE *file,
>> + my_bool do_reinit,
>> + size_t count)
>> {
>> - size_t bytes_in_cache;
>> + size_t curr_write, bytes_in_cache;
>>DBUG_ENTER("my_b_copy_to_file");
>>
>>/* Reinit the cache to read from the beginning of the cache */
>> - if (reinit_io_cache(cache, READ_CACHE, 0L, FALSE, FALSE))
>> + if (do_reinit && reinit_io_cache(cache, READ_CACHE, 0L, FALSE, FALSE))
>
> generally, when there's a function that is always called with a
> constant (compile-time) argument, I prefer to split the code
> compile-time too, if it isn't too much trouble. In this case it would
> mean a new function like
>
> int my_b_copy_all_to_file(IO_CACHE *cache, FILE *file)
> {
> if (reinit_io_cache(cache, READ_CACHE, 0L, FALSE, FALSE)
> return 1;
> return my_b_copy_to_file(cache, file, SIZE_T_MAX);
> }
>
why is the preference specifically?
I looked around to find something like
https://en.wikipedia.org/wiki/Compile_time_function_execution
... but I somewhat doubt you meant that. Or did you really?
Now to my acknowledgments and comments...
>
> There are no major problems, but see comments below. There're few
> suggestions how to simplify the code.
>
> On Nov 05, Andrei Elkin wrote:
>> revision-id: d282f5c55609469cd74d7390f70c7d922c778711
>> (mariadb-10.1.35-93-gd282f5c5560)
>> parent(s): 2a576f71c5d3c7aacef564e5b1251f83bde48f51
>> author: Andrei Elkin
>> committer: Andrei Elkin
>> timestamp: 2018-10-21 23:42:00 +0300
>> message:
>>
>> MDEV-10963 Fragmented BINLOG query
>>
>> diff --git
>> a/mysql-test/suite/binlog/t/binlog_mysqlbinlog_row_frag.test
>> b/mysql-test/suite/binlog/t/binlog_mysqlbinlog_row_frag.test
>> new file mode 100644
>> index 000..bdf41c94c76
>> --- /dev/null
>> +++ b/mysql-test/suite/binlog/t/binlog_mysqlbinlog_row_frag.test
>> @@ -0,0 +1,50 @@
>> +--source include/have_debug.inc
>> +--source include/have_log_bin.inc
>> +--source include/have_binlog_format_row.inc
>
> you don't need to include have_log_bin, if you include
> have_binlog_format_row.
ack
>
>> +
>> +--let $MYSQLD_DATADIR= `select @@datadir`
>> +--let $max_size=1024
>> +
>> +CREATE TABLE t (a TEXT);
>> +# events of interest are guaranteed to stay in 01 log
>> +RESET MASTER;
>> +--eval INSERT INTO t SET a=repeat('a', $max_size)
>
> eh? why did you do it with let/eval instead of a simple sql statement?
> you don't use $max_size anywhere else.
left inadvertently from a previous patch.
>
>> +SELECT a from t into @a;
>> +FLUSH LOGS;
>> +DELETE FROM t;
>> +
>> +--exec $MYSQL_BINLOG --debug-binlog-row-event-max-encoded-size=256
>> $MYSQLD_DATADIR/master-bin.01 >
>> $MYSQLTEST_VARDIR/tmp/mysqlbinlog.sql
>> +
>> +--let $assert_text= BINLOG is fragmented
>> +--let $assert_select= BINLOG @binlog_fragment_0, @binlog_fragment_1
>> +--let $assert_count= 1
>> +--let $assert_file= $MYSQLTEST_VARDIR/tmp/mysqlbinlog.sql
>> +--source include/assert_grep.inc
>
> no, please, use search_pattern_in_file.inc instead.
ack
>
>> +
>> +--exec $MYSQL test < $MYSQLTEST_VARDIR/tmp/mysqlbinlog.sql
>> +
>> +SELECT a LIKE @a as 'true' FROM t;
>> +SELECT @binlog_fragment_0, @binlog_fragment_1 as 'NULL';
>
> that makes no sense, @binlog_fragment_0 and _1 were set in a separate
> client session. You cannot test whether they were cleared or not there,
> by looking at the values here
You caught me here! :-).
In the new patch I relocate the check in the other test file.
>
>> +
>> +# improper syntax error
>> +--echo BINLOG number-of-fragments must be exactly two
>> +--error ER_PARSE_ERROR
>> +BINLOG @binlog_fragment;
>> +--error ER_PARSE_ERROR
>> +BINLOG @binlog_fragment, @binlog_fragment, @binlog_fragment;
>> +
>> +# corrupted fragments error check (to the expected error code notice,
>> +# the same error code occurs in a similar unfragmented case)
>> +SET @binlog_fragment_0='012345';
>> +SET @binlog_fragment_1='012345';
>> +--error ER_SYNTAX_ERROR
>> +BINLOG @binlog_fragment_0, @binlog_fragment_1;
>> +
>> +# Not existing fragment is not allowed
>> +SET @binlog_fragment_0='012345';
>> +--error ER_WRONG_TYPE_FOR_VAR
>> +BINLOG @binlog_fragment_0, @binlog_fragment_not_exist;
>> +
>> +--echo # Cleanup
>> +--remove_file $MYSQLTEST_VARDIR/tmp/mysqlbinlog.sql
>> +DROP TABLE t;
>> diff --git a/mysys/mf_iocache2.c b/mysys/mf_iocache2.c
>> --- a/mysys/mf_iocache2.c
>> +++ b/mysys/mf_iocache2.c
>> @@ -22,52 +22,53 @@
>> #include
>> #include
>>
>> -/*
>> - Copy contents of an IO_CACHE to a
