List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:April 28 2010 2:23pm
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849
WL#2649
View as plain text  
Hello,

Alexander Barkov a écrit, Le 28.04.2010 14:13:
> #At file:///home/bar/mysql-bzr/mysql-trunk-bugfixing/ based on
> revid:dao-gang.qu@stripped
> 
>  3025 Alexander Barkov	2010-04-28
>       Bug#52849 [Com]: datetime index not work
>       Problem: after introduction of "WL#2649 Number-to-string conversions"
>       
>       This query:
>         SET NAMES cp850; -- Or any other non-latin1 ASCII-based character set
>         SELECT * FROM t1
>         WHERE datetime_column='2010-01-01 00:00:00'
>       started to add extra character set conversion: 
>         SELECT * FROM t1
>         WHERE CONVERT(datetime_column USING cp850)='2010-01-01 00:00:00';
>       
>       so index on DATETIME column was not used anymore.
>       
>       Fix:
>       avoid convertion of NUMERIC/DATETIME items
>       (i.e. those with derivation DERIVATION_NUMERIC).
> 
>     modified:
>       mysql-test/r/type_datetime.result
>       mysql-test/t/type_datetime.test
>       sql/item.cc

I'm reviewing this but being quite ignorant of the concepts here, I have 
only questions so far.

> === modified file 'sql/item.cc'
> --- a/sql/item.cc	2010-04-19 08:27:46 +0000
> +++ b/sql/item.cc	2010-04-28 12:04:20 +0000
> @@ -1796,6 +1796,20 @@ bool agg_item_set_converter(DTCollation 
>                                    &dummy_offset))
>        continue;
>  
> +    /*
> +      No needs to add converter if an "arg" is NUMERIC or DATETIME
> +      value (which is pure ASCII) and at the same time target DTCollation
> +      is ASCII-compatible. For example, no needs to rewrite:
> +        SELECT * FROM t1 WHERE datetime_field = '2010-01-01';
> +      to
> +        SELECT * FROM t1 WHERE CONVERT(datetime_field USING cs) = '2010-01-01';
> +    */
> +    if ((*arg)->collation.derivation == DERIVATION_NUMERIC &&

> +        (*arg)->collation.repertoire == MY_REPERTOIRE_ASCII &&

is this test needed? Isn't the first one (about derivations) enough? 
What is a repertoire (ok it's a French word but what does it mean here 
;-)? What does MY_REPERTOIRE_ASCII mean?
There exists MY_REPERTOIRE_NUMERIC, should it be used instead?

> +        !((*arg)->collation.collation->state & MY_CS_NONASCII) &&

a member named "collation" in an object named "collation" 
(collation.collation) - what is it about?

I have the impression that the 3 first tests are about the number/date's 
collation? do we need to test all three?

> +        !(coll.collation->state & MY_CS_NONASCII))

this one is about the target collation it seems.

Can't the entire if() just be:
if !((*arg)->collation.collation->state & MY_CS_NONASCII) &&
!(coll.collation->state & MY_CS_NONASCII))
{
  //it's all ASCII
  continue;
}
I guess no, but why?

> +      continue;
> +
>      if (!(conv= (*arg)->safe_charset_converter(coll.collation)) &&
>          ((*arg)->collation.repertoire == MY_REPERTOIRE_ASCII))
>        conv= new Item_func_conv_charset(*arg, coll.collation, 1);

Thanks!

-- 
Mr. Guilhem Bichot <guilhem@stripped>
Sun Microsystems / MySQL, Lead Software Engineer
Bordeaux, France
www.sun.com / www.mysql.com
Thread
bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849 WL#2649Alexander Barkov28 Apr
  • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849WL#2649Guilhem Bichot28 Apr
    • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849WL#2649Alexander Barkov28 Apr
      • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849WL#2649Guilhem Bichot29 Apr
        • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849WL#2649Alexander Barkov29 Apr
          • Re: bzr commit into mysql-trunk-bugfixing branch (bar:3025) Bug#52849WL#2649Alexander Barkov29 Apr