Hi Sven
I agree with you, we should not bother to change it again, thank you for
you work.
On 2008-02-19 Tue 18:09 +0100，Sven Sandberg wrote：
> 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...
>
