From: Alexander Barkov
Date: March 4 2011 4:23pm
Subject: Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341
 Bug#11764503
List-Archive: http://lists.mysql.com/commits/132498
Message-Id: <4D7111FD.5050102@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 04.03.2011 09:34:
>> 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.
> 
> Great!
> 
>> I have only one small doubt:
>> usually we don't use enums for flag-alike things.
> 
> Yes, I used to think this way. But in a review I was informed that enum 
> is actually suited for flag-alike things. I have just seen such usage in 
> Stroustrup's "the c++ programming language" book, where he writes 
> something like
> enum enum_x { enum_member1, enum_member2 };
> enum_x var= enum_x(enum_member1 | enum_member2);
> This works because the standard guarantees that if the maximum value of 
> the enumeration is N, which takes k bits to represent, then the enum 
> accepts any value representable on max k bits. So bit-wise operations work.
> The enum_x() is necessary: there is implicit conversion from enum to 
> int, but not from int to enum; this is intentional because enum_x may 
> not hold all of the int range (it only holds max k bits). The result of 
> '|' is an int, we convert explicitely to enum_x.
> 
>> On the other hand, enums are better than just integer #defines,
>> e.g. they make gdb sessions easier.
> 
> yes
> 
>> 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) ?
> 
> I'd prefer not to do so. My idea was that we define flags, and then the 
> combinations are left to the caller. Defining combinations in the enum, 
> somehow gives an impression that this combination is special, as 
> "predefined by the enum writer". And we'd have to find a good name for 
> it. And if later people use other combinations, they would also add them 
> to the enum, whose definition would become hard to read.
> 
Ok. Sound reasonable.

Thank you for review and a good suggestion!
Pushed into mysql-5.5 and mysql-trunk.
Greetings.