List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:November 15 2010 4:25pm
Subject:Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to
3229)
View as plain text  
Hello again,

Jorgen Loland a écrit, Le 09.11.2010 13:55:
> Review of changesets up to 3218:
> 

> opt_trace-t.cc
> --------------
> 1) There are many add_str calls in the test. These could cover more
>    code if they were called with different parameters, specifically
>      a) with length parameter

done

>      b) with another character set/escaped characters

I added a separate subtest at the end of the unit test, which feeds 129 
utf8 characters into add_str_escaped(). The first 128 are ASCII (among 
them the control characters 0-32 which come out properly escaped); the 
last one is a two-byte character (ä).
As for testing "another character set": add_str*() calls expect UTF8 
input, so if I put something else in the test it's violating the API.

There is a real-life case of such violation, already tested in 
optimizer_trace_charset.test, where we hit BUG#57341 and the trace has 
to deal with non-utf8 chars in the input. But this can happen only if 
the query is not in utf8 (character_set_client != utf8); 
character_set_client is announced to the optimizer trace via 
set_query(). If the trace knows that character_set_client != utf8, it 
expects to meet BUG#57341, so does an additional pass over the trace and 
replaces all non-utf8 chars with '?'. I have just added this to the unit 
test.

Note that the additional pass above is triggered by the fact that 
set_query() was told about a non-utf8 charset.
We don't do this pass if the query is said to be in utf8, as a speed 
optimization (if character_set_client is utf8, BUG#57341 does not happen).
So, if we pass non-utf8 chars after doing set_query(..., ..., 
system_charset_info), the trace system will not do the additional pass.

> 2) Test get_current_struct() by add()-ing to it

there is already an invokation of OPT_TRACE in test portion named 
"normal_usage", and OPT_TRACE is a macro which expands to 
get_current_struct(); is that enough?

> 
> 3) Test set_query() and then print it with "normal" and "strange"
>    character set.

For the optimizer trace, the query is always returned as it was entered, 
there is no conversion, as it goes into a separate column of 
OPTIMIZER_TRACE having its own charset (character_set_client), it does 
not go into the JSON trace.
So I just added a test that a query is returned as entered, but as there 
is no charset conversion, there is no point in testing many charsets (I 
have added a test which uses utf8 in set_query() and one which uses 
latin1 in set_query())

I will soon be sending a commit for your review.

Thread
bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to 3229) Guilhem Bichot22 Oct
Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Jorgen Loland26 Oct
  • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Guilhem Bichot5 Nov
    • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Jorgen Loland10 Nov
  • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Jorgen Loland9 Nov
    • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Guilhem Bichot12 Nov
      • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Jorgen Loland15 Nov
        • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Guilhem Bichot15 Nov
    • Re: bzr push into mysql-next-mr-bugfixing branch (guilhem:3228 to3229)Guilhem Bichot15 Nov