Hi Sergei! On Tue, Aug 23, 2016 at 4:35 PM, Sergei Golubchik <s...@mariadb.org> wrote: > > Hi, Sachin! > > On Aug 22, Sachin Setia wrote: > > Hi Sergei! > > > > Actually I completed the work on update and delete. Now they will use > > index for looking up records. > > > > But I am thinking I have done a lot of changes in optimizer which may > > break it, and also there are lots of queries where my code does not > > work, fixing this might take a long amount of time. I am thinking of > > a change in my existing code :- > > Suppose a table t1 > > create table t1 (a blob, b blob, c blob, unique(a,b,c)); > > In current code, for query like there will a KEY with only one > > keypart which points to field DB_ROW_HASH_1. > > It was okay for normal updates, insert and delete, but in the case > > of where optimization I have do a lot of stuff, first to match field > > (like in add_key_part), then see whether all the fields in hash_str > > are present in where or not, then create keys by calculating hash. I > > do this by checking the HA_UNIQUE_HASH flag in KEY, but this also > > makes (I think) optimizer code bad because of too much dependence. > > Also I need to patch get_mm_parts and get_mm_leaf function, which I > > think should not be patched. > > Later today I'll know exactly what you mean, when I'll finish > reviewing your optimizer changes. > > But for now, let's say I agree on a general principle :) > Optimizer is kinda complex and fragile, so it's good to avoid doing many > changes in it - the effect might be difficult to predict. > > > I am thinking of a another approach to this problem at server level > > instead of having just one keypart we can have 1+3 keypart. Last three > > keyparts will be for field a, b, c and first one for > > DB_ROW_HASH_1.These will be only at server level not at storage level. > > key_info->key_part will point at keypart containing field a, while > > key_part having field DB_ROW_HASH_1 will -1 index. By this way I do > > not have to patch more of optimizer code. But there is one problem, > > what should be the length of key_part? I am thinking of it equal to > > field->pack_length(), this would not work because while creating keys > > optimizer calls get_key_image() (which is real data so can exceed > > pack_lenght() in case of blob), so to get this work I have to patch > > optimizer where it calls get_key_image() and see if key is > > HA_UNIQUE_HASH. If yes then instead of get_key_image just use > > memcpy(key, field->ptr(), field->pack_length()); > > this wont copy the actual data, but we do not need actual data. I will > > patch handler methods like ha_index_read, ha_index_idx_read, > > multi_range_read_info_const basically handler methods which are > > related to index or range search. In these methods i need to > > calculate hash, which I can calculate from key_ptr but key_ptr doe > > not have actual data(in case of blobs etc).So to get the data for > > hash, I will make a field clone of (a,b,c etc) but there ptr will > > point in key_ptr. Then field->val_str() method will work simply and i > > can calculate hash. And also I can compare returned result with > > actual key in handler method itself. > > What do you think of this approach ? > > Looks simpler, agree. The length of the keypart should not matter, > because it should never be used. May be it would be good to set it to -1 > as it might help to catch errors (where it is erroneously used).
I think it should , because we make buffer size according to key_ptr->length. For example , this code at test_quick_select (param.min_key= (uchar*)alloc_root(&alloc,max_key_len) here max_key_length is sum of lengths of all key_part->store_length and also in function get_mm_leaf we use field->get_key_image(str+maybe_null, key_part->length, key_part->image_type); So I think length will matter. > > I didn't understand why you need to clone fields though :( Ohh , this is actually just for reading data from key_ptr, because key_ptr does not have direct data , so i thought that i have to make a clone of field and then set its ptr. But I guess this function would work. inline void move_field(uchar *ptr_arg,uchar *null_ptr_arg,uchar null_bit_arg) { ptr=ptr_arg; null_ptr=null_ptr_arg; null_bit=null_bit_arg; } > > > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org Regards sachin _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp