List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:March 4 2011 8:53am
Subject:Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341
Bug#11764503
View as plain text  
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.

Thread
bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Alexander Barkov1 Mar
  • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Guilhem Bichot1 Mar
    • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Alexander Barkov1 Mar
      • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Guilhem Bichot2 Mar
        • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Alexander Barkov3 Mar
  • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Guilhem Bichot3 Mar
    • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Alexander Barkov4 Mar
      • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Guilhem Bichot4 Mar
        • Re: bzr commit into mysql-5.5 branch (alexander.barkov:3357) Bug#57341Bug#11764503Alexander Barkov4 Mar