On 11/01/2010 03:21 PM, Olav Sandstå wrote:
> Hi Jørgen,
> Thanks for looking at the patch and for suggesting an alternative
> approach. See some comments inline.
> I have tried out this suggestion and it seems to work. One extra thing
> that needed to be implemented for this to work was that the handler's
> end_range.key is on "server format" and is normally used for comparing
> against values already converted to "server format" and stored in the
> record0 buffer. Your suggested code is working directly against the
> content of the index entry which is on MyISAM format. So to make this
> work I had to convert ("pack") the end_range.key to MyISAM format
> before doing any comparision.
> I have a working prototype based on your suggestion that seems to work
> fine with all tests in the main test suite passing.
>> The problem is that mi_rkey.c does not include ha_myisam.h, so we'll need to
>> connect these somehow. This can, e.g., be done by having a function pointer
>> (as done with index_cond_func) or the end_range key in the info object.
> A third alternative for this would be to move this check into the
> existing index_cond_func_myisam() function in ha_myisam.cc. This would
> then require this function to get access to the info->lastkey member.
> If we solve this bug by adding this extra code for check of "end
> range" to handle the special case for BLOB this would lead to
> duplication of "end range" check. We would have one in
> index_cond_func_myisam() that does this using the content of the
> record0 buffer and one that do this check using the last read myisam
> key entry. I think if we go for this solution we should convert all
> testing of "end range" related to ICP to just use the last read key on
> MyISAM format (some more work).
> I think it would be good to do all these changes. Still, I think the
> best way to solve this bug is to use my original approach to fix the
> ICP bug and then rather file a feature request bug against MyISAM to
> change the way MyISAM does "end range" check to use your suggested
> approach. Any objections?
No objections. Thanks for the explanation. Original patch approved.
Jørgen Løland | Senior Software Engineer | +47 73842138