Hi Zhenxing,
He Zhenxing wrote:
[...]
>> diff -Nrup a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc
>> --- a/client/mysqlbinlog.cc 2008-02-01 20:28:20 +01:00
>> +++ b/client/mysqlbinlog.cc 2008-02-06 19:13:50 +01:00
[...]
>> @@ -627,6 +643,11 @@ int process_event(PRINT_EVENT_INFO *prin
>> */
>> if (ce)
>> {
>> + /*
>> + We must not convert earlier, since the file is used by
>> + my_open() in Load_log_processor::append().
>> + */
>> + convert_path_to_forward_slashes((char*) ce->fname);
>> ce->print(result_file, print_event_info, TRUE);
>
> I would prefer change the ce->print method instead of modify the
> ce->fname attribute.
>
> This is not a problem now because we don't use ce->fname any more after
> calling ce->print, but this might change in the future. And if we call
> ce->print in some other places in the future, we have to call
> convert_path_to_forward_slashes before that too. So IMHO, I think modify
> ce->print method might be better.
I agree, that would have been a much better way to code it. However, the
bug is fixed and the patch pushed, and I'd have to reboot to windows to
test it again, so I'd rather forget about it for the moment :-)
I don't mean to be stubborn, I am very glad that you comment on this
kind of things. But I think the whole design of the file load events is
wrong, and would rather abandon them (and use row-based logging for all
LOAD DATA INFILE instead). See, e.g., BUG#34283. Therefore, I'd rather
spend as little time as possible with those events. But feel free to fix
it yourself, or leave a /* @todo: ... */ in the code...
--
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com
