From: Alexander Barkov
Date: March 4 2011 8:34am
Subject: Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341
 Bug#11764503
List-Archive: http://lists.mysql.com/commits/132430
Message-Id: <4D70A415.7090907@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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.