List:Falcon Storage Engine« Previous MessageNext Message »
From:Lars-Erik Bjørk Date:February 10 2009 8:04pm
Subject:Re: bug#34478
View as plain text  
I have committed a patch based on this, what does people think?

http://lists.mysql.com/commits/65802

/Lars-Erik


On Feb 10, 2009, at 1:20 PM, Vladislav Vaintroub wrote:

> checkTail()/padding do not make sense to me. I would skip it or  
> better yet,
> delete and see what happens.
>
>
>> -----Original Message-----
>> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped]
>> Sent: Tuesday, February 10, 2009 1:16 PM
>> To: Vladislav Vaintroub
>> Cc: Kevin.Lewis@stripped; 'FalconDev'
>> Subject: Re: bug#34478
>>
>> Looking at the compare methods for the IndexKey class (used in
>> IndexPage::indexMerge), none of them pad the shorter key to the  
>> length
>> of the longer. They only compare on record number if the keys are  
>> equal
>> up to the length of the shortest, and it is partial. Otherwise it
>> compares on key length.
>>
>> Would this make sense for the DeferredIndex::compare methods as well?
>> Skip the checkTails() calls? Or will this make types with a different
>> pad character than 0x00 sort incorrectly.
>>
>> The following method with the (for some reason) commented out line
>> leads
>> me to believe that wen only use 0x00 for the mentioned padding  
>> anyway:
>>
>> void Field::setCollation(Collation *newCollation)
>> {
>>    collation = newCollation;
>>    //indexPadByte = collation->getPadChar();
>>    indexPadByte = 0;
>> }
>>
>>
>> I am not so familiar with what is the correct padding character and
>> when
>> to pad etc. in all cases for every type, although the topic is very
>> fascinating ;)
>>
>> Btw, Kevin, I tried removing the padding, and that seems to fix the  
>> bug
>> you linked to (bug#33190), when enabling the assert again. So the  
>> bugs
>> are definitively closely related.
>>
>> Best,
>>
>> Lars-Erik
>>
>> Vladislav Vaintroub wrote:
>>> I guess varbinary does not have any padding and this is the clue...
>>>
>>> The compare function for index keys does not exist as of today.
>>>
>>> It could look like
>>>
>>> diff = memcmp(a, b, min(length_a,length_b));
>>> if (diff == 0)
>>>  diff = (length_a > length_b)?1: ((length_a < length_b):-1:0);
>>>
>>>
>>> What makes me wonder in this bug is that IndexPage is not healthy
>> either.
>>>
>>> This could be effect of optimization that has been in index code
>> since ever:
>>> when merging a new key, use the fact that keys in log record are
>> presorted -
>>>
>>> findInsertionPoint does not search from the start of the page, but
>> from the
>>> position where prior key has been inserted.
>>> And if keys are wrongly presorted, IndexPage could be easily
>> corrupted too.
>>>
>>>
>>>
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Kevin.Lewis@stripped [mailto:Kevin.Lewis@stripped]
>>>> Sent: Monday, February 09, 2009 9:46 PM
>>>> To: Lars-Erik Bjørk
>>>> Cc: FalconDev
>>>> Subject: Re: bug#34478
>>>>
>>>> What about making the DeferredIndex class sort things exactly like
>> the
>>>> IndexKeys read from index pages?  Can it use the same compare
>> function?
>>>>
>>>> Lars-Erik Bjørk wrote:
>>>>
>>>>> Hi all!
>>>>>
>>>>> I don't have the the battery capacity to write a long mail now
>> (lucky
>>>>> you!) :) I analyzed bug#34478 (
>>>>>
>>>> http://bugs.mysql.com/bug.php?id=34478)
>>>>
>>>>> which also narrows down to the use of 0x00 in index keys (this  
>>>>> time
>>>>>
>>>> as
>>>>
>>>>> pad bytes in the deferred index). Does anyone have an opinion on
>> how
>>>>> this bug can be avoided in a nice and correct fashion? Does it  
>>>>> make
>>>>>
>>>> any
>>>>
>>>>> sense to pad with 0x00?
>>>>>
>>>>> 2 minutes capacity left, sorry the short mail, please see the
>> report
>>>>>
>>>> for
>>>>
>>>>> a bit more elaborate version :)
>>>>>
>>>>> Best,
>>>>>
>>>>> Lars-Erik
>>>>>
>>>>>
>>>> --
>>>> Falcon Storage Engine Mailing List
>>>> For list archives: http://lists.mysql.com/falcon
>>>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>>>>
>>>
>>>
>>>
>>> --
>>> Falcon Storage Engine Mailing List
>>> For list archives: http://lists.mysql.com/falcon
>>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>> erik.bjork@stripped
>>>
>>>
>>
>>
>> --
>> Falcon Storage Engine Mailing List
>> For list archives: http://lists.mysql.com/falcon
>> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>
>
>
> --
> Falcon Storage Engine Mailing List
> For list archives: http://lists.mysql.com/falcon
> To unsubscribe:    http://lists.mysql.com/falcon?unsub=1
>

Thread
bug#34478Lars-Erik Bjørk9 Feb
  • Re: bug#34478Kevin Lewis9 Feb
    • RE: bug#34478Vladislav Vaintroub9 Feb
      • Re: bug#34478Lars-Erik Bjørk10 Feb
        • RE: bug#34478Vladislav Vaintroub10 Feb
          • Re: bug#34478Lars-Erik Bjørk10 Feb
            • RE: bug#34478Vladislav Vaintroub10 Feb
              • Re: bug#34478Lars-Erik Bjørk10 Feb
                • RE: bug#34478Vladislav Vaintroub10 Feb
                  • Re: bug#34478Lars-Erik Bjørk11 Feb