maria-developers team mailing list archive

[Sachin Setia <sachinsetia1001@xxxxxxxxx>]

Hello Sergei!
I am stuck at one problem
consider table t1(a blob , b blob , unique(a,b));
although select * from t1 where a= 12 and b= 23 works
but consider the case like select * from t1 where ( a= 12  or a=45 ) and
(b= 23 or b=45 ) does not works
and also update and delete using long index does not work
simple query like
delete/ update table t1 where a=1 and b=3;
does not works

The reason is that because these query uses test_quick_select function
which does not recognize hash index basically in get_mm_parts it always
return false because it compare db_row_hash_1 to a and b i solved this
problem
but now the problems
1. first how to assure we have both column a, and b is in where currently i
do this as crawl through tree->key[i]->
next_key_part untill next_key_part is null this works for simple case like
a= 1 and b=2 but i am not sure how will i do this in
conditions like
((a =  1 or a =34) and (b=34 or b=33)) or (b=34)
^^^^^^^^^^^^^^^^^^^^^^^
in this condition before or past it is okay to use hash but for b=34 we
should not use hash index but do not know how to do

2. when SEL_TREE is evaluated ?

3. where should i evaluate hash i can do that but main problem is that i
need to preserve original data  like a=1 and b=3
etc  because hash does not guaranty same.

4 there is one more problem  in test_quick_select

there is one code

if ((range_trp= get_key_scans_params(&param, tree, FALSE, TRUE,

                                           best_read_time)))

in the case of hash key before reaching this function we must have to
calculate hash after this it will work
but i guess upto this point our tree is not evaluated , i do not know

 please review branch
https://github.com/SachinSetiya/server/tree/unique_index_where_up_de  and
tell me what should
i do and also review the orignal branch  unique_index_sachin  i added more
test cases and reinsert also works

On Sun, Aug 14, 2016 at 1:46 PM, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Sachin!
>
> I'm reviewing.
> Here I only comment on your reply to the previous review.
>
> > >> +longlong  Item_func_hash::val_int()
> > >> +{
> > >> +  unsigned_flag= true;
> > >> +  ulong nr1= 1,nr2= 4;
> > >> +  CHARSET_INFO *cs;
> > >> +  for(uint i= 0;i<arg_count;i++)
> > >> +  {
> > >> +    String * str = args[i]->val_str();
> > >> +    if(args[i]->null_value)
> > >> +    {
> > >> +      null_value= 1;
> > >> +      return 0;
> > >> +    }
> > >> +    cs= str->charset();
> > >> +    uchar l[4];
> > >> +    int4store(l,str->length());
> > >> +    cs->coll->hash_sort(cs,l,sizeof(l), &nr1, &nr2);
> > > looks good, but use my_charset_binary for the length.
> > did not get it :(
>
> You use
>
>   cs->coll->hash_sort(cs,l,sizeof(l), &nr1, &nr2);
>
> to sort the length, the byte value of str->length().
> This is binary data, you should have
>
>   cs= my_charset_binary;
i do not find any variable name my_charset_binary but i used

my_charset_utf8_bin is it ok ?

>
> but you have
>
>   cs= str->charset();
>
> for example, it can be latin1_general_ci, and then 'a' and 'A' will be
> considered equal. But 'a' is the string length 97, while 'A' is string
> length 65. String lengths are binary data, they do not consist of
> "letters" or "characters", so you need to use binary charset for
> lengths, but str->charset() for actual string data.
>
> > >> +    cs->coll->hash_sort(cs, (uchar *)str->ptr(), str->length(),
&nr1, &nr2);
>
> ^^^ here cs= str->charset() is correct.
>
> > >> +  }
> > >> +  return   (longlong)nr1;
> > >> +}
> > >> +
> > >> +
> > >> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> > >> index e614692..4872d20 100644
> > >> --- a/sql/sql_yacc.yy
> > >> +++ b/sql/sql_yacc.yy
> > >>           | COMMENT_SYM TEXT_STRING_sys { Lex->last_field->comment=
$2; }
> > >> +        | HIDDEN
> > >> +          {
> > >> +              LEX *lex =Lex;
> > > no need to do that ^^^ you can simply write
Lex->last_field->field_visibility=...
> > change but in sql_yacc.yy this type of code is use everywhere
>
> Yes :) Many years ago Lex was defined to be current_thd->lex and on some
> platforms current_thd (which is pthread_getspecific) is expensive, so we
> were trying to avoid using it when possible. That's why Lex was saved in
> a local variable.
>
> But now (also, for many years) Lex is YYTHD->Lex, where YYTHD is the
> argument of the MYSQLyyparse function, not pthread_getspecific.
> So there is no need to avoid Lex anymore. Nobody bothered to fix the old
> code, though.
>
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx