Hi Guilhem,
Guilhem Bichot wrote:
> Hello,
>
> Alexander Barkov a écrit, Le 01.03.2011 11:24:
>> #At file:///home/bar/mysql-bzr/mysql-5.5.b57341/ based on
>> revid:tor.didriksen@stripped
>>
>> 3357 Alexander Barkov 2011-03-01
>> Bug#11764503 (Bug#57341) Query in EXPLAIN EXTENDED shows wrong
>> characters
>> @ mysql-test/r/ctype_latin1.result
>> @ mysql-test/r/ctype_utf8.result
>> @ mysql-test/t/ctype_latin1.test
>> @ mysql-test/t/ctype_utf8.test
>> Adding tests
>
> Ok to push.
> I had looked at Item_string::print() earlier and found that its
> execution flow is hard to read.
> So here is a proposal (incremental over your patch) which makes
> Item_string::print() hopefully clearer.
> I also added comments which explain the reasons behind what is done for
> QT_EXPLAIN (something I understood after a gdb session :-).
>
> http://lists.mysql.com/commits/132357
I like you proposal in http://lists.mysql.com/commits/132357.
It's much easier to read this way.
I have only one small doubt:
usually we don't use enums for flag-alike things.
On the other hand, enums are better than just integer #defines,
e.g. they make gdb sessions easier.
Enum value (QT_TO_SYSTEM_CHARSET | QT_WITHOUT_INTRODUCERS)
is not defined. Shouldn't we at least define a enum value for
(QT_TO_SYSTEM_CHARSET | QT_WITHOUT_INTRODUCERS) ?
Thanks!
>
> I hope you'll like it. If you don't want it, your patch is still ok to
> push, but could you please at least incorporate my comment about
> QT_EXPLAIN ("Because we wrote an introducer" etc)?
>
> Thanks.
